Re: [PATCH v2 07/13] drm/msm/dpu: Add SM6350 support

[Dmitry Baryshkov]

On 21/04/2023 02:05, Konrad Dybcio wrote:
> On 21.04.2023 00:41, Dmitry Baryshkov wrote:
> > On 21/04/2023 01:31, Konrad Dybcio wrote:
> > > Add SM6350 support to the DPU1 driver to enable display output.
> > >
> > > Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxxxxxx>
> > > Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> > > ---
> [...]
>
> > > +
> > > +static const struct dpu_sspp_cfg sm6350_sspp[] = {
> > > +    SSPP_BLK("sspp_0", SSPP_VIG0, 0x4000, 0x1f8, VIG_SC7180_MASK,
> > > +         sc7180_vig_sblk_0, 0,  SSPP_TYPE_VIG, DPU_CLK_CTRL_VIG0),
> > > +    SSPP_BLK("sspp_8", SSPP_DMA0, 0x24000, 0x1f8, DMA_SDM845_MASK,
> > > +         sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> > > +    SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1f8, DMA_CURSOR_SDM845_MASK,
> > > +         sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR0),
> >
> > DPU_CLK_CTRL_DMA0

This is DPU_CLK_CTRL_DMA1, I made a typo

> > > +    SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1f8, DMA_CURSOR_SDM845_MASK,
> > > +         sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_CURSOR1),
> >
> > DPU_CLK_CTRL_DMA2
> _DMA1?

And this is DMA2, I was correct.


> > > +};
> > > +
>
> > > +static const struct dpu_qos_lut_entry sm6350_qos_linear_macrotile[] = {
> > > +    {.fl = 0, .lut = 0x0011223344556677 },
> > > +    {.fl = 0, .lut = 0x0011223445566777 },
> >
> > Do we need two equal entries here?
> Hmm.. looks like the SDE driver dropped the fill level
> logic in 4.19 times and that might have thrown me off
> when porting this Since the [0] entry has what looks
> like a lower LUT value, should I give it .fl=1?
>
>
> > Also please push the qos to the dpu_hw_catalog.c, I want to take another look at these structures and it is easier if all of them are beneath one's eyes.
> Will do.
>
> > > +};
> > > +
> > > +static const struct dpu_perf_cfg sm6350_perf_data = {
> > > +    .max_bw_low = 4200000,
> > > +    .max_bw_high = 5100000,
> > > +    .min_core_ib = 2500000,
> > > +    .min_llcc_ib = 0,
> > > +    .min_dram_ib = 1600000,
> > > +    .min_prefill_lines = 35,
> > > +    /* TODO: confirm danger_lut_tbl */
> > > +    .danger_lut_tbl = {0xffff, 0xffff, 0x0, 0x0, 0xffff},
> [...]
>
>
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h
> > > @@ -320,6 +320,8 @@ enum dpu_qos_lut_usage {
> > >        DPU_QOS_LUT_USAGE_LINEAR,
> > >        DPU_QOS_LUT_USAGE_MACROTILE,
> > >        DPU_QOS_LUT_USAGE_NRT,
> > > +    DPU_QOS_LUT_USAGE_CWB,
> > > +    DPU_QOS_LUT_USAGE_MACROTILE_QSEED,
> >
> > This should probably be removed. It would be nice to clean these things up, but not as a part of sm6350.
> Well, I won't be able to fill in the danger LUT table otherwise!
>
> Konrad
> > >        DPU_QOS_LUT_USAGE_MAX,
> > >    };
> > >    @@ -880,6 +882,7 @@ extern const struct dpu_mdss_cfg dpu_sc8180x_cfg;
> > >    extern const struct dpu_mdss_cfg dpu_sm8250_cfg;
> > >    extern const struct dpu_mdss_cfg dpu_sc7180_cfg;
> > >    extern const struct dpu_mdss_cfg dpu_sm6115_cfg;
> > > +extern const struct dpu_mdss_cfg dpu_sm6350_cfg;
> > >    extern const struct dpu_mdss_cfg dpu_qcm2290_cfg;
> > >    extern const struct dpu_mdss_cfg dpu_sm8350_cfg;
> > >    extern const struct dpu_mdss_cfg dpu_sc7280_cfg;
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > index 0e7a68714e9e..46be7ad8d615 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c
> > > @@ -1286,6 +1286,7 @@ static const struct of_device_id dpu_dt_match[] = {
> > >        { .compatible = "qcom,sc8180x-dpu", .data = &dpu_sc8180x_cfg, },
> > >        { .compatible = "qcom,sc8280xp-dpu", .data = &dpu_sc8280xp_cfg, },
> > >        { .compatible = "qcom,sm6115-dpu", .data = &dpu_sm6115_cfg, },
> > > +    { .compatible = "qcom,sm6350-dpu", .data = &dpu_sm6350_cfg, },
> > >        { .compatible = "qcom,sm8150-dpu", .data = &dpu_sm8150_cfg, },
> > >        { .compatible = "qcom,sm8250-dpu", .data = &dpu_sm8250_cfg, },
> > >        { .compatible = "qcom,sm8350-dpu", .data = &dpu_sm8350_cfg, },

--
With best wishes
Dmitry