| Message ID | 20240915161245.30296-1-jason-jh.lin@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] drm/mediatek: ovl: Add fmt_support_man for MT8192 and MT8195 | expand |
Il 15/09/24 18:12, Jason-JH.Lin ha scritto: > OVL_CON_CLRFMT_MAN is an configuration for extending color format > settings of DISP_REG_OVL_CON(n). > It will change some of the original color format settings. > > Take the settings of (3 << 12) for example. > - If OVL_CON_CLRFMT_MAN = 0 means OVL_CON_CLRFMT_RGBA8888. > - If OVL_CON_CLRFMT_MAN = 1 means OVL_CON_CLRFMT_PARGB8888. > > Since OVL_CON_CLRFMT_MAN is not supported on previous SoCs, > It breaks the OVL color format setting of MT8173. > So add fmt_support_man to the driver data of MT8192 and MT8195 > to solve the downgrade problem. > > Fixes: a3f7f7ef4bfe ("drm/mediatek: Support "Pre-multiplied" blending in OVL") > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 43 ++++++++++++++++++++----- > 1 file changed, 35 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > index 89b439dcf3a6..7b053ca25b10 100644 > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c > @@ -70,10 +70,33 @@ > #define OVL_CON_CLRFMT_UYVY (4 << 12) > #define OVL_CON_CLRFMT_YUYV (5 << 12) > #define OVL_CON_MTX_YUV_TO_RGB (6 << 16) > -#define OVL_CON_CLRFMT_PARGB8888 ((3 << 12) | OVL_CON_CLRFMT_MAN) > -#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_RGB_SWAP) > -#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_BYTE_SWAP) > -#define OVL_CON_CLRFMT_PRGBA8888 (OVL_CON_CLRFMT_PABGR8888 | OVL_CON_BYTE_SWAP) > + > +#define OVL_CON_CLRFMT_PARGB8888(ovl) ({ \ > + struct mtk_disp_ovl *_ovl = (ovl); \ > + (_ovl->data->fmt_support_man ? \ > + ((3 << 12) | OVL_CON_CLRFMT_MAN) : OVL_CON_CLRFMT_ARGB8888); \ > +}) Isn't it just simpler to pass an "extended input format selection" flag to these macros, instead of the entire mtk_disp_ovl structure? Something like... #define OVL_CON_CLRFMT_PARGB8888(fmt_support_man) ...or alternatively, it's probably simply more straightforward to assign function pointers to struct mtk_disp_ovl_data: `.fmt_convert = ovl_fmt_convert_mt8173` for MT8173-style controllers and `.fmt_convert = ovl_fmt_convert_mt8195` for MT8195-style controllers, so that you can simply call `con = ovl->data->fmt_convert(ovl, fmt, blend_mode)` in mtk_ovl_layer_config(). The thing is - those macros seem to be unnecessarily complicated, and it's at least impacting on readability... :-) Cheers, Angelo > + > +#define OVL_CON_CLRFMT_PABGR8888(ovl) ({ \ > + struct mtk_disp_ovl *_ovl = (ovl); \ > + (_ovl->data->fmt_support_man ? \ > + (OVL_CON_CLRFMT_PARGB8888(_ovl) | OVL_CON_RGB_SWAP) : OVL_CON_CLRFMT_ABGR8888); \ > +}) > + > +#define OVL_CON_CLRFMT_PBGRA8888(ovl) ({ \ > + struct mtk_disp_ovl *_ovl = (ovl); \ > + (_ovl->data->fmt_support_man ? \ > + (OVL_CON_CLRFMT_PARGB8888(_ovl) | OVL_CON_BYTE_SWAP) : \ > + OVL_CON_CLRFMT_BGRA8888); \ > +}) > + > +#define OVL_CON_CLRFMT_PRGBA8888(ovl) ({ \ > + struct mtk_disp_ovl *_ovl = (ovl); \ > + (_ovl->data->fmt_support_man ? \ > + (OVL_CON_CLRFMT_PABGR8888(_ovl) | OVL_CON_BYTE_SWAP) : \ > + OVL_CON_CLRFMT_RGBA8888); \ > +}) > + > #define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ > 0 : OVL_CON_CLRFMT_RGB) > #define OVL_CON_CLRFMT_RGB888(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ > @@ -144,6 +167,7 @@ struct mtk_disp_ovl_data { > unsigned int gmc_bits; > unsigned int layer_nr; > bool fmt_rgb565_is_0; > + bool fmt_support_man; > bool smi_id_en; > bool supports_afbc; > const u32 *formats; > @@ -410,28 +434,28 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt, > case DRM_FORMAT_RGBA1010102: > return blend_mode == DRM_MODE_BLEND_COVERAGE ? > OVL_CON_CLRFMT_RGBA8888 : > - OVL_CON_CLRFMT_PRGBA8888; > + OVL_CON_CLRFMT_PRGBA8888(ovl); > case DRM_FORMAT_BGRX8888: > case DRM_FORMAT_BGRA8888: > case DRM_FORMAT_BGRX1010102: > case DRM_FORMAT_BGRA1010102: > return blend_mode == DRM_MODE_BLEND_COVERAGE ? > OVL_CON_CLRFMT_BGRA8888 : > - OVL_CON_CLRFMT_PBGRA8888; > + OVL_CON_CLRFMT_PBGRA8888(ovl); > case DRM_FORMAT_XRGB8888: > case DRM_FORMAT_ARGB8888: > case DRM_FORMAT_XRGB2101010: > case DRM_FORMAT_ARGB2101010: > return blend_mode == DRM_MODE_BLEND_COVERAGE ? > OVL_CON_CLRFMT_ARGB8888 : > - OVL_CON_CLRFMT_PARGB8888; > + OVL_CON_CLRFMT_PARGB8888(ovl); > case DRM_FORMAT_XBGR8888: > case DRM_FORMAT_ABGR8888: > case DRM_FORMAT_XBGR2101010: > case DRM_FORMAT_ABGR2101010: > return blend_mode == DRM_MODE_BLEND_COVERAGE ? > OVL_CON_CLRFMT_ABGR8888 : > - OVL_CON_CLRFMT_PABGR8888; > + OVL_CON_CLRFMT_PABGR8888(ovl); > case DRM_FORMAT_UYVY: > return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB; > case DRM_FORMAT_YUYV: > @@ -662,6 +686,7 @@ static const struct mtk_disp_ovl_data mt8192_ovl_driver_data = { > .gmc_bits = 10, > .layer_nr = 4, > .fmt_rgb565_is_0 = true, > + .fmt_support_man = true, > .smi_id_en = true, > .formats = mt8173_formats, > .num_formats = ARRAY_SIZE(mt8173_formats), > @@ -672,6 +697,7 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = { > .gmc_bits = 10, > .layer_nr = 2, > .fmt_rgb565_is_0 = true, > + .fmt_support_man = true, > .smi_id_en = true, > .formats = mt8173_formats, > .num_formats = ARRAY_SIZE(mt8173_formats), > @@ -682,6 +708,7 @@ static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = { > .gmc_bits = 10, > .layer_nr = 4, > .fmt_rgb565_is_0 = true, > + .fmt_support_man = true, > .smi_id_en = true, > .supports_afbc = true, > .formats = mt8195_formats,
Hi, On 2024-09-15 19:12 +03:00, Jason-JH.Lin wrote: > OVL_CON_CLRFMT_MAN is an configuration for extending color format > settings of DISP_REG_OVL_CON(n). > It will change some of the original color format settings. > > Take the settings of (3 << 12) for example. > - If OVL_CON_CLRFMT_MAN = 0 means OVL_CON_CLRFMT_RGBA8888. > - If OVL_CON_CLRFMT_MAN = 1 means OVL_CON_CLRFMT_PARGB8888. > > Since OVL_CON_CLRFMT_MAN is not supported on previous SoCs, > It breaks the OVL color format setting of MT8173. > So add fmt_support_man to the driver data of MT8192 and MT8195 > to solve the downgrade problem. > > Fixes: a3f7f7ef4bfe ("drm/mediatek: Support "Pre-multiplied" blending in OVL") > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 43 ++++++++++++++++++++----- > 1 file changed, 35 insertions(+), 8 deletions(-) Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com> Thanks!
> > + > > +#define OVL_CON_CLRFMT_PARGB8888(ovl) ({ \ > > + struct mtk_disp_ovl *_ovl = (ovl); \ > > + (_ovl->data->fmt_support_man ? \ > > + ((3 << 12) | OVL_CON_CLRFMT_MAN) : OVL_CON_CLRFMT_ARGB8888); \ > > +}) > > Isn't it just simpler to pass an "extended input format selection" > flag to > these macros, instead of the entire mtk_disp_ovl structure? > > Something like... > > #define OVL_CON_CLRFMT_PARGB8888(fmt_support_man) > > ...or alternatively, it's probably simply more straightforward to > assign > function pointers to struct mtk_disp_ovl_data: > > `.fmt_convert = ovl_fmt_convert_mt8173` for MT8173-style controllers > and > `.fmt_convert = ovl_fmt_convert_mt8195` for MT8195-style controllers, > > so that you can simply call `con = ovl->data->fmt_convert(ovl, fmt, > blend_mode)` > in mtk_ovl_layer_config(). > > The thing is - those macros seem to be unnecessarily complicated, and > it's at > least impacting on readability... :-) OK, I'll add `.fmt_convert = mtk_ovl_fmt_convert_with_blend` to the driver data for MT8192 and MT8195 and add `.fmt_convert = mtk_ovl_fmt_convert` to the driver data for other SoCs. Regards, Jason-JH.Lin > > Cheers, > Angelo > > >
diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c index 89b439dcf3a6..7b053ca25b10 100644 --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c @@ -70,10 +70,33 @@ #define OVL_CON_CLRFMT_UYVY (4 << 12) #define OVL_CON_CLRFMT_YUYV (5 << 12) #define OVL_CON_MTX_YUV_TO_RGB (6 << 16) -#define OVL_CON_CLRFMT_PARGB8888 ((3 << 12) | OVL_CON_CLRFMT_MAN) -#define OVL_CON_CLRFMT_PABGR8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_RGB_SWAP) -#define OVL_CON_CLRFMT_PBGRA8888 (OVL_CON_CLRFMT_PARGB8888 | OVL_CON_BYTE_SWAP) -#define OVL_CON_CLRFMT_PRGBA8888 (OVL_CON_CLRFMT_PABGR8888 | OVL_CON_BYTE_SWAP) + +#define OVL_CON_CLRFMT_PARGB8888(ovl) ({ \ + struct mtk_disp_ovl *_ovl = (ovl); \ + (_ovl->data->fmt_support_man ? \ + ((3 << 12) | OVL_CON_CLRFMT_MAN) : OVL_CON_CLRFMT_ARGB8888); \ +}) + +#define OVL_CON_CLRFMT_PABGR8888(ovl) ({ \ + struct mtk_disp_ovl *_ovl = (ovl); \ + (_ovl->data->fmt_support_man ? \ + (OVL_CON_CLRFMT_PARGB8888(_ovl) | OVL_CON_RGB_SWAP) : OVL_CON_CLRFMT_ABGR8888); \ +}) + +#define OVL_CON_CLRFMT_PBGRA8888(ovl) ({ \ + struct mtk_disp_ovl *_ovl = (ovl); \ + (_ovl->data->fmt_support_man ? \ + (OVL_CON_CLRFMT_PARGB8888(_ovl) | OVL_CON_BYTE_SWAP) : \ + OVL_CON_CLRFMT_BGRA8888); \ +}) + +#define OVL_CON_CLRFMT_PRGBA8888(ovl) ({ \ + struct mtk_disp_ovl *_ovl = (ovl); \ + (_ovl->data->fmt_support_man ? \ + (OVL_CON_CLRFMT_PABGR8888(_ovl) | OVL_CON_BYTE_SWAP) : \ + OVL_CON_CLRFMT_RGBA8888); \ +}) + #define OVL_CON_CLRFMT_RGB565(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ 0 : OVL_CON_CLRFMT_RGB) #define OVL_CON_CLRFMT_RGB888(ovl) ((ovl)->data->fmt_rgb565_is_0 ? \ @@ -144,6 +167,7 @@ struct mtk_disp_ovl_data { unsigned int gmc_bits; unsigned int layer_nr; bool fmt_rgb565_is_0; + bool fmt_support_man; bool smi_id_en; bool supports_afbc; const u32 *formats; @@ -410,28 +434,28 @@ static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt, case DRM_FORMAT_RGBA1010102: return blend_mode == DRM_MODE_BLEND_COVERAGE ? OVL_CON_CLRFMT_RGBA8888 : - OVL_CON_CLRFMT_PRGBA8888; + OVL_CON_CLRFMT_PRGBA8888(ovl); case DRM_FORMAT_BGRX8888: case DRM_FORMAT_BGRA8888: case DRM_FORMAT_BGRX1010102: case DRM_FORMAT_BGRA1010102: return blend_mode == DRM_MODE_BLEND_COVERAGE ? OVL_CON_CLRFMT_BGRA8888 : - OVL_CON_CLRFMT_PBGRA8888; + OVL_CON_CLRFMT_PBGRA8888(ovl); case DRM_FORMAT_XRGB8888: case DRM_FORMAT_ARGB8888: case DRM_FORMAT_XRGB2101010: case DRM_FORMAT_ARGB2101010: return blend_mode == DRM_MODE_BLEND_COVERAGE ? OVL_CON_CLRFMT_ARGB8888 : - OVL_CON_CLRFMT_PARGB8888; + OVL_CON_CLRFMT_PARGB8888(ovl); case DRM_FORMAT_XBGR8888: case DRM_FORMAT_ABGR8888: case DRM_FORMAT_XBGR2101010: case DRM_FORMAT_ABGR2101010: return blend_mode == DRM_MODE_BLEND_COVERAGE ? OVL_CON_CLRFMT_ABGR8888 : - OVL_CON_CLRFMT_PABGR8888; + OVL_CON_CLRFMT_PABGR8888(ovl); case DRM_FORMAT_UYVY: return OVL_CON_CLRFMT_UYVY | OVL_CON_MTX_YUV_TO_RGB; case DRM_FORMAT_YUYV: @@ -662,6 +686,7 @@ static const struct mtk_disp_ovl_data mt8192_ovl_driver_data = { .gmc_bits = 10, .layer_nr = 4, .fmt_rgb565_is_0 = true, + .fmt_support_man = true, .smi_id_en = true, .formats = mt8173_formats, .num_formats = ARRAY_SIZE(mt8173_formats), @@ -672,6 +697,7 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = { .gmc_bits = 10, .layer_nr = 2, .fmt_rgb565_is_0 = true, + .fmt_support_man = true, .smi_id_en = true, .formats = mt8173_formats, .num_formats = ARRAY_SIZE(mt8173_formats), @@ -682,6 +708,7 @@ static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = { .gmc_bits = 10, .layer_nr = 4, .fmt_rgb565_is_0 = true, + .fmt_support_man = true, .smi_id_en = true, .supports_afbc = true, .formats = mt8195_formats,
OVL_CON_CLRFMT_MAN is an configuration for extending color format settings of DISP_REG_OVL_CON(n). It will change some of the original color format settings. Take the settings of (3 << 12) for example. - If OVL_CON_CLRFMT_MAN = 0 means OVL_CON_CLRFMT_RGBA8888. - If OVL_CON_CLRFMT_MAN = 1 means OVL_CON_CLRFMT_PARGB8888. Since OVL_CON_CLRFMT_MAN is not supported on previous SoCs, It breaks the OVL color format setting of MT8173. So add fmt_support_man to the driver data of MT8192 and MT8195 to solve the downgrade problem. Fixes: a3f7f7ef4bfe ("drm/mediatek: Support "Pre-multiplied" blending in OVL") Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> --- drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 43 ++++++++++++++++++++----- 1 file changed, 35 insertions(+), 8 deletions(-)