Re: [PATCH 07/11] drm/msm/dpu: add sspp cursor blocks to msm8998 hw catalog

[Dmitry Baryshkov]

On Thu, 20 Apr 2023 at 10:06, Arnaud Vrac <avrac@xxxxxxxxxx> wrote:
>
> Le jeu. 20 avr. 2023 à 01:10, Dmitry Baryshkov
> <dmitry.baryshkov@xxxxxxxxxx> a écrit :
> >
> > On 19/04/2023 17:41, Arnaud Vrac wrote:
> > > Now that cursor sspp blocks can be used for cursor planes, enable them
> > > on msm8998. The dma sspp blocks that were assigned to cursor planes can
> > > now be used for overlay planes instead.
> >
> > While the change is correct, there is more about it. Composers, using
> > universal planes, will see this plane too. They have no obligations to
> > use it only for the cursor. At the minimum could you please extend the
> > plane_atomic_check to check for the plane dimensions for the CURSOR pipes?
>
> Hum, I had assumed the generic atomic checks would already do this,

Atomic will have these checks for the legacy cursor API (using the
mode_config.cursor_width/cursor_height that you have added). But I
don't think there is a generic API for telling the core 'this plane is
slightly limited'.

Fortunately, once the virtual planes land and are taught about the
SSPP_CURSOR peculiarities, it should not matter, since the driver will
know that it should not select these pipes in the inappropriate cases.

> but it's not the case. I'll add the check when the pipe is of type
> SSPP_CURSOR in another patch coming before, thanks.
>
> >
> > For this change:
> >
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
> >
> > >
> > > Signed-off-by: Arnaud Vrac <avrac@xxxxxxxxxx>
> > > ---
> > >   .../drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h    |  8 +++--
> > >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c     | 34 ++++++++++++++++++++++
> > >   2 files changed, 40 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > > index b07e8a9941f79..7de393b0f91d7 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_3_0_msm8998.h
> > > @@ -90,10 +90,14 @@ static const struct dpu_sspp_cfg msm8998_sspp[] = {
> > >               sdm845_dma_sblk_0, 1, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA0),
> > >       SSPP_BLK("sspp_9", SSPP_DMA1, 0x26000, 0x1ac, DMA_MSM8998_MASK,
> > >               sdm845_dma_sblk_1, 5, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA1),
> > > -     SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > > +     SSPP_BLK("sspp_10", SSPP_DMA2, 0x28000, 0x1ac, DMA_MSM8998_MASK,
> > >               sdm845_dma_sblk_2, 9, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA2),
> > > -     SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > > +     SSPP_BLK("sspp_11", SSPP_DMA3, 0x2a000, 0x1ac, DMA_MSM8998_MASK,
> > >               sdm845_dma_sblk_3, 13, SSPP_TYPE_DMA, DPU_CLK_CTRL_DMA3),
> > > +     SSPP_BLK("sspp_12", SSPP_CURSOR0, 0x34000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > > +             msm8998_cursor_sblk_0, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR0),
> > > +     SSPP_BLK("sspp_13", SSPP_CURSOR1, 0x36000, 0x1ac, DMA_CURSOR_MSM8998_MASK,
> > > +             msm8998_cursor_sblk_1, 10, SSPP_TYPE_CURSOR, DPU_CLK_CTRL_CURSOR1),
> > >   };
> > >
> > >   static const struct dpu_lm_cfg msm8998_lm[] = {
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > index 8d5d782a43398..f34fa704936bc 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > > @@ -242,6 +242,22 @@ static const uint32_t wb2_formats[] = {
> > >       DRM_FORMAT_XBGR4444,
> > >   };
> > >
> > > +static const uint32_t cursor_formats[] = {
> > > +     DRM_FORMAT_ARGB8888,
> > > +     DRM_FORMAT_ABGR8888,
> > > +     DRM_FORMAT_RGBA8888,
> > > +     DRM_FORMAT_BGRA8888,
> > > +     DRM_FORMAT_XRGB8888,
> > > +     DRM_FORMAT_ARGB1555,
> > > +     DRM_FORMAT_ABGR1555,
> > > +     DRM_FORMAT_RGBA5551,
> > > +     DRM_FORMAT_BGRA5551,
> > > +     DRM_FORMAT_ARGB4444,
> > > +     DRM_FORMAT_ABGR4444,
> > > +     DRM_FORMAT_RGBA4444,
> > > +     DRM_FORMAT_BGRA4444,
> > > +};
> > > +
> > >   /*************************************************************
> > >    * SSPP sub blocks config
> > >    *************************************************************/
> > > @@ -300,6 +316,19 @@ static const uint32_t wb2_formats[] = {
> > >       .virt_num_formats = ARRAY_SIZE(plane_formats), \
> > >       }
> > >
> > > +#define _CURSOR_SBLK(num) \
> > > +     { \
> > > +     .maxdwnscale = SSPP_UNITY_SCALE, \
> > > +     .maxupscale = SSPP_UNITY_SCALE, \
> > > +     .smart_dma_priority = 0, \
> > > +     .src_blk = {.name = STRCAT("sspp_src_", num), \
> > > +             .id = DPU_SSPP_SRC, .base = 0x00, .len = 0x150,}, \
> > > +     .format_list = cursor_formats, \
> > > +     .num_formats = ARRAY_SIZE(cursor_formats), \
> > > +     .virt_format_list = cursor_formats, \
> > > +     .virt_num_formats = ARRAY_SIZE(cursor_formats), \
> > > +     }
> > > +
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_0 =
> > >                               _VIG_SBLK("0", 0, DPU_SSPP_SCALER_QSEED3);
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_1 =
> > > @@ -309,6 +338,11 @@ static const struct dpu_sspp_sub_blks msm8998_vig_sblk_2 =
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_3 =
> > >                               _VIG_SBLK("3", 0, DPU_SSPP_SCALER_QSEED3);
> > >
> > > +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_0 =
> > > +                             _CURSOR_SBLK("12");
> > > +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_1 =
> > > +                             _CURSOR_SBLK("13");
> > > +
> > >   static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 = {
> > >       .rot_maxheight = 1088,
> > >       .rot_num_formats = ARRAY_SIZE(rotation_v2_formats),
> > >
> >
> > --
> > With best wishes
> > Dmitry
> >



-- 
With best wishes
Dmitry