On Fri, 8 Dec 2023 at 19:53, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > On 12/8/2023 3:44 AM, Dmitry Baryshkov wrote: > > On Fri, 8 Dec 2023 at 07:07, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> 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@xxxxxxxxxxx> > >> --- > >> .../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