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