Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230419-dpu-tweaks-v1-0-d1bac46db075@freebox.fr>
 <20230419-dpu-tweaks-v1-7-d1bac46db075@freebox.fr> <405ff057-e4da-3f2f-b860-ce2eeacaab94@linaro.org>
 <CAG9NU68aLLZ0KGNmsirzm5RtGw6CC_i=+kTwyfhy+bjSkRTO4Q@mail.gmail.com>
In-Reply-To: <CAG9NU68aLLZ0KGNmsirzm5RtGw6CC_i=+kTwyfhy+bjSkRTO4Q@mail.gmail.com>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date:   Thu, 20 Apr 2023 11:47:40 +0300
Message-ID: <CAA8EJppa86WWE0tkUaFG4JpfKwr9husvgHWkCVjRbWiF=Ei7aQ@mail.gmail.com>
Subject: Re: [PATCH 07/11] drm/msm/dpu: add sspp cursor blocks to msm8998 hw catalog
To:     Arnaud Vrac <avrac@freebox.fr>
Cc:     Rob Clark <robdclark@gmail.com>,
        Abhinav Kumar <quic_abhinavk@quicinc.com>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, linux-arm-msm@vger.kernel.org,
        dri-devel@lists.freedesktop.org, freedreno@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, 20 Apr 2023 at 10:06, Arnaud Vrac <avrac@freebox.fr> wrote:
>
> Le jeu. 20 avr. 2023 =C3=A0 01:10, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> a =C3=A9crit :
> >
> > On 19/04/2023 17:41, Arnaud Vrac wrote:
> > > Now that cursor sspp blocks can be used for cursor planes, enable the=
m
> > > on msm8998. The dma sspp blocks that were assigned to cursor planes c=
an
> > > 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 pip=
es?
>
> 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@linaro.org>
> >
> > >
> > > Signed-off-by: Arnaud Vrac <avrac@freebox.fr>
> > > ---
> > >   .../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[] =
=3D {
> > >               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_MSM89=
98_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_MSM89=
98_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_MS=
M8998_MASK,
> > > +             msm8998_cursor_sblk_0, 2, SSPP_TYPE_CURSOR, DPU_CLK_CTR=
L_CURSOR0),
> > > +     SSPP_BLK("sspp_13", SSPP_CURSOR1, 0x36000, 0x1ac, DMA_CURSOR_MS=
M8998_MASK,
> > > +             msm8998_cursor_sblk_1, 10, SSPP_TYPE_CURSOR, DPU_CLK_CT=
RL_CURSOR1),
> > >   };
> > >
> > >   static const struct dpu_lm_cfg msm8998_lm[] =3D {
> > > 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[] =3D {
> > >       DRM_FORMAT_XBGR4444,
> > >   };
> > >
> > > +static const uint32_t cursor_formats[] =3D {
> > > +     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[] =3D {
> > >       .virt_num_formats =3D ARRAY_SIZE(plane_formats), \
> > >       }
> > >
> > > +#define _CURSOR_SBLK(num) \
> > > +     { \
> > > +     .maxdwnscale =3D SSPP_UNITY_SCALE, \
> > > +     .maxupscale =3D SSPP_UNITY_SCALE, \
> > > +     .smart_dma_priority =3D 0, \
> > > +     .src_blk =3D {.name =3D STRCAT("sspp_src_", num), \
> > > +             .id =3D DPU_SSPP_SRC, .base =3D 0x00, .len =3D 0x150,},=
 \
> > > +     .format_list =3D cursor_formats, \
> > > +     .num_formats =3D ARRAY_SIZE(cursor_formats), \
> > > +     .virt_format_list =3D cursor_formats, \
> > > +     .virt_num_formats =3D ARRAY_SIZE(cursor_formats), \
> > > +     }
> > > +
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_0 =3D
> > >                               _VIG_SBLK("0", 0, DPU_SSPP_SCALER_QSEED=
3);
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_1 =3D
> > > @@ -309,6 +338,11 @@ static const struct dpu_sspp_sub_blks msm8998_vi=
g_sblk_2 =3D
> > >   static const struct dpu_sspp_sub_blks msm8998_vig_sblk_3 =3D
> > >                               _VIG_SBLK("3", 0, DPU_SSPP_SCALER_QSEED=
3);
> > >
> > > +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_0 =3D
> > > +                             _CURSOR_SBLK("12");
> > > +static const struct dpu_sspp_sub_blks msm8998_cursor_sblk_1 =3D
> > > +                             _CURSOR_SBLK("13");
> > > +
> > >   static const struct dpu_rotation_cfg dpu_rot_sc7280_cfg_v2 =3D {
> > >       .rot_maxheight =3D 1088,
> > >       .rot_num_formats =3D ARRAY_SIZE(rotation_v2_formats),
> > >
> >
> > --
> > With best wishes
> > Dmitry
> >


--=20
With best wishes
Dmitry