| Message ID | 20240106-fd-migrate-mdp5-v3-1-3d2750378063@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm: provide migration path from MDP5 to DPU driver | expand |
On 1/5/2024 3:34 PM, Dmitry Baryshkov wrote: > Older (mdp5) platforms do not use per-SoC compatible strings. Instead > they use a single compat entry 'qcom,mdss'. To facilitate migrating > these platforms to the DPU driver provide a way to generate the MDSS / > UBWC data at runtime, when the DPU driver asks for it. > > It is not possible to generate this data structure at the probe time, > since some platforms might not have MDP_CLK enabled, which makes reading > HW_REV register return 0. > I would have expected a crash if clock was not enabled and we tried to access the hw_rev register. > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > --- > drivers/gpu/drm/msm/msm_mdss.c | 51 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 51 insertions(+) > > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c > index 455b2e3a0cdd..566a5dd5b8e8 100644 > --- a/drivers/gpu/drm/msm/msm_mdss.c > +++ b/drivers/gpu/drm/msm/msm_mdss.c > @@ -3,6 +3,7 @@ > * Copyright (c) 2018, The Linux Foundation > */ > > +#include <linux/bitfield.h> > #include <linux/clk.h> > #include <linux/delay.h> > #include <linux/interconnect.h> > @@ -213,6 +214,49 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss) > } > } > > +#define MDSS_HW_MAJ_MIN GENMASK(31, 16) > + > +#define MDSS_HW_MSM8996 0x1007 > +#define MDSS_HW_MSM8937 0x100e > +#define MDSS_HW_MSM8956 0x1010 This should be 0x100B in the docs I see. > +#define MDSS_HW_MSM8998 0x3000 > +#define MDSS_HW_SDM660 0x3002 > +#define MDSS_HW_SDM630 0x3003 > + > +/* > + * MDP5 platforms use generic qcom,mdp5 compat string, so we have to generate this data > + */ > +static const struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct msm_mdss *mdss) > +{ > + struct msm_mdss_data *data; > + u32 hw_rev; > + > + data = devm_kzalloc(mdss->dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return NULL; > + > + hw_rev = readl_relaxed(mdss->mmio + HW_REV); > + hw_rev = FIELD_GET(MDSS_HW_MAJ_MIN, hw_rev); > + > + if (hw_rev == MDSS_HW_MSM8996 || > + hw_rev == MDSS_HW_MSM8937 || > + hw_rev == MDSS_HW_MSM8956 || > + hw_rev == MDSS_HW_MSM8998 || > + hw_rev == MDSS_HW_SDM660 || > + hw_rev == MDSS_HW_SDM630) { > + data->ubwc_dec_version = UBWC_1_0; > + data->ubwc_enc_version = UBWC_1_0; > + } > + > + if (hw_rev == MDSS_HW_MSM8996 || > + hw_rev == MDSS_HW_MSM8998) > + data->highest_bank_bit = 2; > + else > + data->highest_bank_bit = 1; > + > + return data; > +} > + > const struct msm_mdss_data *msm_mdss_get_mdss_data(struct device *dev) > { > struct msm_mdss *mdss; > @@ -222,6 +266,13 @@ const struct msm_mdss_data *msm_mdss_get_mdss_data(struct device *dev) > > mdss = dev_get_drvdata(dev); > > + /* > + * We could not do it at the probe time, since hw revision register was > + * not readable. Fill data structure now for the MDP5 platforms. > + */ > + if (!mdss->mdss_data && mdss->is_mdp5) > + mdss->mdss_data = msm_mdss_generate_mdp5_mdss_data(mdss); > + > return mdss->mdss_data; > } > >
On Wed, 7 Feb 2024 at 20:25, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote: > > > > On 1/5/2024 3:34 PM, Dmitry Baryshkov wrote: > > Older (mdp5) platforms do not use per-SoC compatible strings. Instead > > they use a single compat entry 'qcom,mdss'. To facilitate migrating > > these platforms to the DPU driver provide a way to generate the MDSS / > > UBWC data at runtime, when the DPU driver asks for it. > > > > It is not possible to generate this data structure at the probe time, > > since some platforms might not have MDP_CLK enabled, which makes reading > > HW_REV register return 0. > > > > I would have expected a crash if clock was not enabled and we tried to > access the hw_rev register. No, for all the platforms I tested it returns 0 instead. However this doesn't make any difference, we don't read the register in MDP5 case until all clocks are enabled. > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> > > --- > > drivers/gpu/drm/msm/msm_mdss.c | 51 ++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 51 insertions(+) > > > > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c > > index 455b2e3a0cdd..566a5dd5b8e8 100644 > > --- a/drivers/gpu/drm/msm/msm_mdss.c > > +++ b/drivers/gpu/drm/msm/msm_mdss.c > > @@ -3,6 +3,7 @@ > > * Copyright (c) 2018, The Linux Foundation > > */ > > > > +#include <linux/bitfield.h> > > #include <linux/clk.h> > > #include <linux/delay.h> > > #include <linux/interconnect.h> > > @@ -213,6 +214,49 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss) > > } > > } > > > > +#define MDSS_HW_MAJ_MIN GENMASK(31, 16) > > + > > +#define MDSS_HW_MSM8996 0x1007 > > +#define MDSS_HW_MSM8937 0x100e > > +#define MDSS_HW_MSM8956 0x1010 > > This should be 0x100B in the docs I see. Yes, I mixed MSM8956 and MSM8953 here. The code support the latter one. > > > +#define MDSS_HW_MSM8998 0x3000 > > +#define MDSS_HW_SDM660 0x3002 > > +#define MDSS_HW_SDM630 0x3003 > > + > > +/* > > + * MDP5 platforms use generic qcom,mdp5 compat string, so we have to generate this data > > + */ > > +static const struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct msm_mdss *mdss) > > +{ > > + struct msm_mdss_data *data; > > + u32 hw_rev; > > + > > + data = devm_kzalloc(mdss->dev, sizeof(*data), GFP_KERNEL); > > + if (!data) > > + return NULL; > > + > > + hw_rev = readl_relaxed(mdss->mmio + HW_REV); > > + hw_rev = FIELD_GET(MDSS_HW_MAJ_MIN, hw_rev); > > + > > + if (hw_rev == MDSS_HW_MSM8996 || > > + hw_rev == MDSS_HW_MSM8937 || > > + hw_rev == MDSS_HW_MSM8956 || > > + hw_rev == MDSS_HW_MSM8998 || > > + hw_rev == MDSS_HW_SDM660 || > > + hw_rev == MDSS_HW_SDM630) { > > + data->ubwc_dec_version = UBWC_1_0; > > + data->ubwc_enc_version = UBWC_1_0; > > + } > > + > > + if (hw_rev == MDSS_HW_MSM8996 || > > + hw_rev == MDSS_HW_MSM8998) > > + data->highest_bank_bit = 2; > > + else > > + data->highest_bank_bit = 1; > > + > > + return data; > > +} > > + > > const struct msm_mdss_data *msm_mdss_get_mdss_data(struct device *dev) > > { > > struct msm_mdss *mdss; > > @@ -222,6 +266,13 @@ const struct msm_mdss_data *msm_mdss_get_mdss_data(struct device *dev) > > > > mdss = dev_get_drvdata(dev); > > > > + /* > > + * We could not do it at the probe time, since hw revision register was > > + * not readable. Fill data structure now for the MDP5 platforms. > > + */ > > + if (!mdss->mdss_data && mdss->is_mdp5) > > + mdss->mdss_data = msm_mdss_generate_mdp5_mdss_data(mdss); > > + > > return mdss->mdss_data; > > } > > > >
diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c index 455b2e3a0cdd..566a5dd5b8e8 100644 --- a/drivers/gpu/drm/msm/msm_mdss.c +++ b/drivers/gpu/drm/msm/msm_mdss.c @@ -3,6 +3,7 @@ * Copyright (c) 2018, The Linux Foundation */ +#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/delay.h> #include <linux/interconnect.h> @@ -213,6 +214,49 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss) } } +#define MDSS_HW_MAJ_MIN GENMASK(31, 16) + +#define MDSS_HW_MSM8996 0x1007 +#define MDSS_HW_MSM8937 0x100e +#define MDSS_HW_MSM8956 0x1010 +#define MDSS_HW_MSM8998 0x3000 +#define MDSS_HW_SDM660 0x3002 +#define MDSS_HW_SDM630 0x3003 + +/* + * MDP5 platforms use generic qcom,mdp5 compat string, so we have to generate this data + */ +static const struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct msm_mdss *mdss) +{ + struct msm_mdss_data *data; + u32 hw_rev; + + data = devm_kzalloc(mdss->dev, sizeof(*data), GFP_KERNEL); + if (!data) + return NULL; + + hw_rev = readl_relaxed(mdss->mmio + HW_REV); + hw_rev = FIELD_GET(MDSS_HW_MAJ_MIN, hw_rev); + + if (hw_rev == MDSS_HW_MSM8996 || + hw_rev == MDSS_HW_MSM8937 || + hw_rev == MDSS_HW_MSM8956 || + hw_rev == MDSS_HW_MSM8998 || + hw_rev == MDSS_HW_SDM660 || + hw_rev == MDSS_HW_SDM630) { + data->ubwc_dec_version = UBWC_1_0; + data->ubwc_enc_version = UBWC_1_0; + } + + if (hw_rev == MDSS_HW_MSM8996 || + hw_rev == MDSS_HW_MSM8998) + data->highest_bank_bit = 2; + else + data->highest_bank_bit = 1; + + return data; +} + const struct msm_mdss_data *msm_mdss_get_mdss_data(struct device *dev) { struct msm_mdss *mdss; @@ -222,6 +266,13 @@ const struct msm_mdss_data *msm_mdss_get_mdss_data(struct device *dev) mdss = dev_get_drvdata(dev); + /* + * We could not do it at the probe time, since hw revision register was + * not readable. Fill data structure now for the MDP5 platforms. + */ + if (!mdss->mdss_data && mdss->is_mdp5) + mdss->mdss_data = msm_mdss_generate_mdp5_mdss_data(mdss); + return mdss->mdss_data; }
Older (mdp5) platforms do not use per-SoC compatible strings. Instead they use a single compat entry 'qcom,mdss'. To facilitate migrating these platforms to the DPU driver provide a way to generate the MDSS / UBWC data at runtime, when the DPU driver asks for it. It is not possible to generate this data structure at the probe time, since some platforms might not have MDP_CLK enabled, which makes reading HW_REV register return 0. Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> --- drivers/gpu/drm/msm/msm_mdss.c | 51 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+)