diff mbox series

[2/6] drm/msm/mdss: generate MDSS data for MDP5 platforms

Message ID 20230905174353.3118648-3-dmitry.baryshkov@linaro.org (mailing list archive)
State Superseded
Headers show
Series drm/msm: provide migration path from MDP5 to DPU driver | expand

Commit Message

Dmitry Baryshkov Sept. 5, 2023, 5:43 p.m. UTC
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 | 37 ++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

Comments

Stephen Boyd Sept. 6, 2023, 10:10 p.m. UTC | #1
Quoting Dmitry Baryshkov (2023-09-05 10:43:49)
> diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> index 348c66b14683..fb6ee93b5abc 100644
> --- a/drivers/gpu/drm/msm/msm_mdss.c
> +++ b/drivers/gpu/drm/msm/msm_mdss.c
> @@ -222,6 +222,36 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss)
>         }
>  }
>
> +static struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct msm_mdss *mdss)

const 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) >> 16;

Or like this

	hw_rev = upper_16_bits(readl_relaxed(...))

> +
> +       if (hw_rev == 0x1007 || /* msm8996 */
> +           hw_rev == 0x100e || /* msm8937 */
> +           hw_rev == 0x1010 || /* msm8953 */
> +           hw_rev == 0x3000 || /* msm8998 */
> +           hw_rev == 0x3002 || /* sdm660 */
> +           hw_rev == 0x3003) { /* sdm630 */

Can we have #defines for hw_revs and drop the comments?

> +               data->ubwc_dec_version = UBWC_1_0;
> +               data->ubwc_enc_version = UBWC_1_0;
> +       }
> +
> +       if (hw_rev == 0x1007 || /* msm8996 */
> +           hw_rev == 0x3000) /* msm8998 */

Then we don't have to worry about these two having typos.

> +               data->highest_bank_bit = 2;
> +       else
> +               data->highest_bank_bit = 1;
>
Dmitry Baryshkov Sept. 6, 2023, 10:18 p.m. UTC | #2
On Thu, 7 Sept 2023 at 01:10, Stephen Boyd <swboyd@chromium.org> wrote:
>
> Quoting Dmitry Baryshkov (2023-09-05 10:43:49)
> > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> > index 348c66b14683..fb6ee93b5abc 100644
> > --- a/drivers/gpu/drm/msm/msm_mdss.c
> > +++ b/drivers/gpu/drm/msm/msm_mdss.c
> > @@ -222,6 +222,36 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss)
> >         }
> >  }
> >
> > +static struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct msm_mdss *mdss)
>
> const mdss?

Ack

>
> > +{
> > +       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) >> 16;
>
> Or like this
>
>         hw_rev = upper_16_bits(readl_relaxed(...))

Ugh. That looks ugly, I'd say. >> 16 is more common.

>
> > +
> > +       if (hw_rev == 0x1007 || /* msm8996 */
> > +           hw_rev == 0x100e || /* msm8937 */
> > +           hw_rev == 0x1010 || /* msm8953 */
> > +           hw_rev == 0x3000 || /* msm8998 */
> > +           hw_rev == 0x3002 || /* sdm660 */
> > +           hw_rev == 0x3003) { /* sdm630 */
>
> Can we have #defines for hw_revs and drop the comments?
>
> > +               data->ubwc_dec_version = UBWC_1_0;
> > +               data->ubwc_enc_version = UBWC_1_0;
> > +       }
> > +
> > +       if (hw_rev == 0x1007 || /* msm8996 */
> > +           hw_rev == 0x3000) /* msm8998 */
>
> Then we don't have to worry about these two having typos.

Ack

>
> > +               data->highest_bank_bit = 2;
> > +       else
> > +               data->highest_bank_bit = 1;
> >
diff mbox series

Patch

diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
index 348c66b14683..fb6ee93b5abc 100644
--- a/drivers/gpu/drm/msm/msm_mdss.c
+++ b/drivers/gpu/drm/msm/msm_mdss.c
@@ -222,6 +222,36 @@  static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss)
 	}
 }
 
+static 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) >> 16;
+
+	if (hw_rev == 0x1007 || /* msm8996 */
+	    hw_rev == 0x100e || /* msm8937 */
+	    hw_rev == 0x1010 || /* msm8953 */
+	    hw_rev == 0x3000 || /* msm8998 */
+	    hw_rev == 0x3002 || /* sdm660 */
+	    hw_rev == 0x3003) { /* sdm630 */
+		data->ubwc_dec_version = UBWC_1_0;
+		data->ubwc_enc_version = UBWC_1_0;
+	}
+
+	if (hw_rev == 0x1007 || /* msm8996 */
+	    hw_rev == 0x3000) /* 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;
@@ -231,6 +261,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;
 }