Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
MIME-Version: 1.0
References: <20231208050641.32582-1-quic_abhinavk@quicinc.com>
 <20231208050641.32582-16-quic_abhinavk@quicinc.com> <CAA8EJpqfCfETawp1up76S6gryO+Q4KxPB3ThwZCe7DCkp=GkBQ@mail.gmail.com>
 <8eea4a8e-0c70-3768-79f0-1a2bfe083ed7@quicinc.com>
In-Reply-To: <8eea4a8e-0c70-3768-79f0-1a2bfe083ed7@quicinc.com>
From:   Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Date:   Fri, 8 Dec 2023 22:45:41 +0200
Message-ID: <CAA8EJpoLzgwEYRcSKZUY1W9KUE9s3WR_bzpA3hmf5X9JGDGutA@mail.gmail.com>
Subject: Re: [PATCH v2 15/16] drm/msm/dpu: introduce separate wb2_format
 arrays for rgb and yuv
To:     Abhinav Kumar <quic_abhinavk@quicinc.com>
Cc:     freedreno@lists.freedesktop.org, Rob Clark <robdclark@gmail.com>,
        Sean Paul <sean@poorly.run>,
        Marijn Suijten <marijn.suijten@somainline.org>,
        David Airlie <airlied@gmail.com>,
        Daniel Vetter <daniel@ffwll.ch>, quic_jesszhan@quicinc.com,
        quic_parellan@quicinc.com, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linux-arm-msm@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, 8 Dec 2023 at 19:53, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
> On 12/8/2023 3:44 AM, Dmitry Baryshkov wrote:
> > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@quicinc.com> wrote:
> >>
> >> Lets rename the existing wb2_formats array wb2_formats_rgb to indicate
> >> that it has only RGB formats and can be used on any chipset having a WB
> >> block.
> >>
> >> Introduce a new wb2_formats_rgb_yuv array to the catalog to
> >> indicate support for YUV formats to writeback in addition to RGB.
> >>
> >> Chipsets which have support for CDM block will use the newly added
> >> wb2_formats_rgb_yuv array.
> >
> > This means that the catalog can go out of sync, if one adds a CDM
> > block but doesn't update wb_formats and vice versa.
> > Can we deduce the format list from the WB code? Is the format list
> > really static or does it change between platforms (please keep msm8996
> > / msm8998 in mind).
> >
>
> Yes this is a valid concern. catalog could potentially go out of sync.
>
> I checked a few chipsets now and the WB formats didnt change among them.
>
> I do need to check more chipsets but downstream does not maintain this
> in devicetree which means we can just move these arrays to WB code
> instead of maintaining them in the catalog.

I think we should be comparing to some of the oldest generations, like
msm8998/sdm660 or ideally even msm8996/37/17/53.

> We will still need to maintain two arrays. One to be used if CDM block
> has been added and the other if not.

Yes.

> I must confess one point though. I have not seen any chipset yet where
> WB block is present but CDM block is not.

I think this was the case for some of mdp5 1.x chips, but according to
my data this is correct for all the platforms that we want to support.

> So at this point, the only purpose of the two arrays will be till the
> point where CDM blk has been added to all the required chipsets in the
> catalog. Then we can drop the RGB only array and maintain the one which
> has all formats.
>
> >>
> >> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> >> ---
> >>   .../msm/disp/dpu1/catalog/dpu_10_0_sm8650.h   |  4 +-
> >>   .../msm/disp/dpu1/catalog/dpu_6_0_sm8250.h    |  4 +-
> >>   .../msm/disp/dpu1/catalog/dpu_6_2_sc7180.h    |  4 +-
> >>   .../msm/disp/dpu1/catalog/dpu_7_2_sc7280.h    |  4 +-
> >>   .../msm/disp/dpu1/catalog/dpu_9_0_sm8550.h    |  4 +-
> >>   .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c    | 37 ++++++++++++++++++-
> >>   6 files changed, 46 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h
> >> index 04d2a73dd942..eb5dfff2ec4f 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_10_0_sm8650.h
> >> @@ -341,8 +341,8 @@ static const struct dpu_wb_cfg sm8650_wb[] = {
> >>                  .name = "wb_2", .id = WB_2,
> >>                  .base = 0x65000, .len = 0x2c8,
> >>                  .features = WB_SM8250_MASK,
> >> -               .format_list = wb2_formats,
> >> -               .num_formats = ARRAY_SIZE(wb2_formats),
> >> +               .format_list = wb2_formats_rgb,
> >> +               .num_formats = ARRAY_SIZE(wb2_formats_rgb),
> >>                  .xin_id = 6,
> >>                  .vbif_idx = VBIF_RT,
> >>                  .maxlinewidth = 4096,
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> >> index 58b0f50518c8..a57d50b1f028 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_0_sm8250.h
> >> @@ -336,8 +336,8 @@ static const struct dpu_wb_cfg sm8250_wb[] = {
> >>                  .name = "wb_2", .id = WB_2,
> >>                  .base = 0x65000, .len = 0x2c8,
> >>                  .features = WB_SM8250_MASK,
> >> -               .format_list = wb2_formats,
> >> -               .num_formats = ARRAY_SIZE(wb2_formats),
> >> +               .format_list = wb2_formats_rgb_yuv,
> >> +               .num_formats = ARRAY_SIZE(wb2_formats_rgb_yuv),
> >>                  .clk_ctrl = DPU_CLK_CTRL_WB2,
> >>                  .xin_id = 6,
> >>                  .vbif_idx = VBIF_RT,
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h
> >> index bcfedfc8251a..7382ebb6e5b2 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_6_2_sc7180.h
> >> @@ -157,8 +157,8 @@ static const struct dpu_wb_cfg sc7180_wb[] = {
> >>                  .name = "wb_2", .id = WB_2,
> >>                  .base = 0x65000, .len = 0x2c8,
> >>                  .features = WB_SM8250_MASK,
> >> -               .format_list = wb2_formats,
> >> -               .num_formats = ARRAY_SIZE(wb2_formats),
> >> +               .format_list = wb2_formats_rgb,
> >> +               .num_formats = ARRAY_SIZE(wb2_formats_rgb),
> >>                  .clk_ctrl = DPU_CLK_CTRL_WB2,
> >>                  .xin_id = 6,
> >>                  .vbif_idx = VBIF_RT,
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> >> index 19c2b7454796..2f153e0b5c6a 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_7_2_sc7280.h
> >> @@ -169,8 +169,8 @@ static const struct dpu_wb_cfg sc7280_wb[] = {
> >>                  .name = "wb_2", .id = WB_2,
> >>                  .base = 0x65000, .len = 0x2c8,
> >>                  .features = WB_SM8250_MASK,
> >> -               .format_list = wb2_formats,
> >> -               .num_formats = ARRAY_SIZE(wb2_formats),
> >> +               .format_list = wb2_formats_rgb_yuv,
> >> +               .num_formats = ARRAY_SIZE(wb2_formats_rgb_yuv),
> >>                  .clk_ctrl = DPU_CLK_CTRL_WB2,
> >>                  .xin_id = 6,
> >>                  .vbif_idx = VBIF_RT,
> >> diff --git a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> >> index bf56265967c0..ad48defa154f 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/catalog/dpu_9_0_sm8550.h
> >> @@ -315,8 +315,8 @@ static const struct dpu_wb_cfg sm8550_wb[] = {
> >>                  .name = "wb_2", .id = WB_2,
> >>                  .base = 0x65000, .len = 0x2c8,
> >>                  .features = WB_SM8250_MASK,
> >> -               .format_list = wb2_formats,
> >> -               .num_formats = ARRAY_SIZE(wb2_formats),
> >> +               .format_list = wb2_formats_rgb,
> >> +               .num_formats = ARRAY_SIZE(wb2_formats_rgb),
> >>                  .xin_id = 6,
> >>                  .vbif_idx = VBIF_RT,
> >>                  .maxlinewidth = 4096,
> >> 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 1be3156cde05..c52cac7a2288 100644
> >> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> >> @@ -202,7 +202,7 @@ static const u32 rotation_v2_formats[] = {
> >>          /* TODO add formats after validation */
> >>   };
> >>
> >> -static const uint32_t wb2_formats[] = {
> >> +static const uint32_t wb2_formats_rgb[] = {
> >>          DRM_FORMAT_RGB565,
> >>          DRM_FORMAT_BGR565,
> >>          DRM_FORMAT_RGB888,
> >> @@ -236,6 +236,41 @@ static const uint32_t wb2_formats[] = {
> >>          DRM_FORMAT_XBGR4444,
> >>   };
> >>
> >> +static const uint32_t wb2_formats_rgb_yuv[] = {
> >> +       DRM_FORMAT_RGB565,
> >> +       DRM_FORMAT_BGR565,
> >> +       DRM_FORMAT_RGB888,
> >> +       DRM_FORMAT_ARGB8888,
> >> +       DRM_FORMAT_RGBA8888,
> >> +       DRM_FORMAT_ABGR8888,
> >> +       DRM_FORMAT_XRGB8888,
> >> +       DRM_FORMAT_RGBX8888,
> >> +       DRM_FORMAT_XBGR8888,
> >> +       DRM_FORMAT_ARGB1555,
> >> +       DRM_FORMAT_RGBA5551,
> >> +       DRM_FORMAT_XRGB1555,
> >> +       DRM_FORMAT_RGBX5551,
> >> +       DRM_FORMAT_ARGB4444,
> >> +       DRM_FORMAT_RGBA4444,
> >> +       DRM_FORMAT_RGBX4444,
> >> +       DRM_FORMAT_XRGB4444,
> >> +       DRM_FORMAT_BGR565,
> >> +       DRM_FORMAT_BGR888,
> >> +       DRM_FORMAT_ABGR8888,
> >> +       DRM_FORMAT_BGRA8888,
> >> +       DRM_FORMAT_BGRX8888,
> >> +       DRM_FORMAT_XBGR8888,
> >> +       DRM_FORMAT_ABGR1555,
> >> +       DRM_FORMAT_BGRA5551,
> >> +       DRM_FORMAT_XBGR1555,
> >> +       DRM_FORMAT_BGRX5551,
> >> +       DRM_FORMAT_ABGR4444,
> >> +       DRM_FORMAT_BGRA4444,
> >> +       DRM_FORMAT_BGRX4444,
> >> +       DRM_FORMAT_XBGR4444,
> >> +       DRM_FORMAT_NV12,
> >> +};
> >> +
> >>   /*************************************************************
> >>    * SSPP sub blocks config
> >>    *************************************************************/
> >> --
> >> 2.40.1
> >>
> >
> >


-- 
With best wishes
Dmitry