diff mbox series

[v6,1/2] drm/mediatek: ovl: Add blend_modes to driver data

Message ID 20240926083526.24629-2-jason-jh.lin@mediatek.com (mailing list archive)
State New
Headers show
Series Fix degradation problem of alpha blending series | expand

Commit Message

Jason-JH.Lin Sept. 26, 2024, 8:35 a.m. UTC 
OVL_CON_CLRFMT_MAN is a 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 previous SoCs did not support OVL_CON_CLRFMT_MAN, this means
that the SoC does not support the premultiplied color format.
It will break the original color format setting of MT8173.

Therefore, the blend_modes is added to the driver data and then
mtk_ovl_fmt_convert() will check the blend_modes to see if
premultiplied supported in current platform.
If it is not supported, use coverage mode to set it to the supported
color formats to solve the degradation problem.

Fixes: a3f7f7ef4bfe ("drm/mediatek: Support "Pre-multiplied" blending in OVL")
Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 41 ++++++++++++++++++++-----
 1 file changed, 34 insertions(+), 7 deletions(-)

Comments

CK Hu (胡俊光) Sept. 26, 2024, 9:30 a.m. UTC | #1 
Hi, Jason:

On Thu, 2024-09-26 at 16:35 +0800, Jason-JH.Lin wrote:
> OVL_CON_CLRFMT_MAN is a 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 previous SoCs did not support OVL_CON_CLRFMT_MAN, this means
> that the SoC does not support the premultiplied color format.
> It will break the original color format setting of MT8173.
> 
> Therefore, the blend_modes is added to the driver data and then
> mtk_ovl_fmt_convert() will check the blend_modes to see if
> premultiplied supported in current platform.
> If it is not supported, use coverage mode to set it to the supported
> color formats to solve the degradation problem.
> 
> Fixes: a3f7f7ef4bfe ("drm/mediatek: Support "Pre-multiplied" blending in OVL")
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>

This version has difference over 50% with previous version.
It's better to drop these tested-by and reviewed-by tag.

> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 41 ++++++++++++++++++++-----
>  1 file changed, 34 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 89b439dcf3a6..0cf7b80f612e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -146,6 +146,7 @@ struct mtk_disp_ovl_data {
>  	bool fmt_rgb565_is_0;
>  	bool smi_id_en;
>  	bool supports_afbc;
> +	const u32 blend_modes;
>  	const u32 *formats;
>  	size_t num_formats;
>  	bool supports_clrfmt_ext;
> @@ -386,14 +387,23 @@ void mtk_ovl_layer_off(struct device *dev, unsigned int idx,
>  		      DISP_REG_OVL_RDMA_CTRL(idx));
>  }
>  
> -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt,
> -				    unsigned int blend_mode)
> +static unsigned int mtk_ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> +					struct mtk_plane_state *state)
>  {
> -	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
> -	 * is defined in mediatek HW data sheet.
> -	 * The alphabet order in XXX is no relation to data
> -	 * arrangement in memory.

I don't know why you drop these comment.
Without this modification,

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> +	unsigned int fmt = state->pending.format;
> +	unsigned int blend_mode = state->base.pixel_blend_mode;
> +
> +	/*
> +	 * For the platforms where OVL_CON_CLRFMT_MAN is defined in the hardware data sheet
> +	 * and supports premultiplied color formats, such as OVL_CON_CLRFMT_PARGB8888.
> +	 *
> +	 * Check blend_modes in the driver data to see if premultiplied mode is supported.
> +	 * If not, use coverage mode instead to set it to the supported color formats.
>  	 */
> +	if (!(ovl->data->blend_modes & BIT(DRM_MODE_BLEND_PREMULTI)) &&
> +	    blend_mode == DRM_MODE_BLEND_PREMULTI)
> +		blend_mode = DRM_MODE_BLEND_COVERAGE;
> +
>  	switch (fmt) {
>  	default:
>  	case DRM_FORMAT_RGB565:
> @@ -471,7 +481,7 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>  		return;
>  	}
>  
> -	con = ovl_fmt_convert(ovl, fmt, blend_mode);
> +	con = mtk_ovl_fmt_convert(ovl, state);
>  	if (state->base.fb) {
>  		con |= OVL_CON_AEN;
>  		con |= state->base.alpha & OVL_CON_ALPHA;
> @@ -626,6 +636,8 @@ static const struct mtk_disp_ovl_data mt2701_ovl_driver_data = {
>  	.gmc_bits = 8,
>  	.layer_nr = 4,
>  	.fmt_rgb565_is_0 = false,
> +	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
>  	.formats = mt8173_formats,
>  	.num_formats = ARRAY_SIZE(mt8173_formats),
>  };
> @@ -635,6 +647,8 @@ static const struct mtk_disp_ovl_data mt8173_ovl_driver_data = {
>  	.gmc_bits = 8,
>  	.layer_nr = 4,
>  	.fmt_rgb565_is_0 = true,
> +	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
>  	.formats = mt8173_formats,
>  	.num_formats = ARRAY_SIZE(mt8173_formats),
>  };
> @@ -644,6 +658,8 @@ static const struct mtk_disp_ovl_data mt8183_ovl_driver_data = {
>  	.gmc_bits = 10,
>  	.layer_nr = 4,
>  	.fmt_rgb565_is_0 = true,
> +	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
>  	.formats = mt8173_formats,
>  	.num_formats = ARRAY_SIZE(mt8173_formats),
>  };
> @@ -653,6 +669,8 @@ static const struct mtk_disp_ovl_data mt8183_ovl_2l_driver_data = {
>  	.gmc_bits = 10,
>  	.layer_nr = 2,
>  	.fmt_rgb565_is_0 = true,
> +	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
>  	.formats = mt8173_formats,
>  	.num_formats = ARRAY_SIZE(mt8173_formats),
>  };
> @@ -663,6 +681,9 @@ static const struct mtk_disp_ovl_data mt8192_ovl_driver_data = {
>  	.layer_nr = 4,
>  	.fmt_rgb565_is_0 = true,
>  	.smi_id_en = true,
> +	.blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
> +		       BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
>  	.formats = mt8173_formats,
>  	.num_formats = ARRAY_SIZE(mt8173_formats),
>  };
> @@ -673,6 +694,9 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
>  	.layer_nr = 2,
>  	.fmt_rgb565_is_0 = true,
>  	.smi_id_en = true,
> +	.blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
> +		       BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
>  	.formats = mt8173_formats,
>  	.num_formats = ARRAY_SIZE(mt8173_formats),
>  };
> @@ -684,6 +708,9 @@ static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
>  	.fmt_rgb565_is_0 = true,
>  	.smi_id_en = true,
>  	.supports_afbc = true,
> +	.blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
> +		       BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
>  	.formats = mt8195_formats,
>  	.num_formats = ARRAY_SIZE(mt8195_formats),
>  	.supports_clrfmt_ext = true,
Jason-JH.Lin Sept. 26, 2024, 9:49 a.m. UTC | #2 
On Thu, 2024-09-26 at 09:30 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
> 
> On Thu, 2024-09-26 at 16:35 +0800, Jason-JH.Lin wrote:
> > OVL_CON_CLRFMT_MAN is a 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 previous SoCs did not support OVL_CON_CLRFMT_MAN, this means
> > that the SoC does not support the premultiplied color format.
> > It will break the original color format setting of MT8173.
> > 
> > Therefore, the blend_modes is added to the driver data and then
> > mtk_ovl_fmt_convert() will check the blend_modes to see if
> > premultiplied supported in current platform.
> > If it is not supported, use coverage mode to set it to the
> > supported
> > color formats to solve the degradation problem.
> > 
> > Fixes: a3f7f7ef4bfe ("drm/mediatek: Support "Pre-multiplied"
> > blending in OVL")
> > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
> > Reviewed-by: AngeloGioacchino Del Regno <
> > angelogioacchino.delregno@collabora.com>
> 
> This version has difference over 50% with previous version.
> It's better to drop these tested-by and reviewed-by tag.

OK, I'll drop this.

> 
> > ---
> >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 41 ++++++++++++++++++++-
> > ----
> >  1 file changed, 34 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > index 89b439dcf3a6..0cf7b80f612e 100644
> > --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> > @@ -146,6 +146,7 @@ struct mtk_disp_ovl_data {
> >  	bool fmt_rgb565_is_0;
> >  	bool smi_id_en;
> >  	bool supports_afbc;
> > +	const u32 blend_modes;
> >  	const u32 *formats;
> >  	size_t num_formats;
> >  	bool supports_clrfmt_ext;
> > @@ -386,14 +387,23 @@ void mtk_ovl_layer_off(struct device *dev,
> > unsigned int idx,
> >  		      DISP_REG_OVL_RDMA_CTRL(idx));
> >  }
> >  
> > -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> > unsigned int fmt,
> > -				    unsigned int blend_mode)
> > +static unsigned int mtk_ovl_fmt_convert(struct mtk_disp_ovl *ovl,
> > +					struct mtk_plane_state *state)
> >  {
> > -	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
> > -	 * is defined in mediatek HW data sheet.
> > -	 * The alphabet order in XXX is no relation to data
> > -	 * arrangement in memory.
> 
> I don't know why you drop these comment.
> Without this modification,

Since we change the MACRO to align this to DRM input format order, this
comment is no longer needed.

I'll send another patch for this.
Thanks for such a quick reply!

Regards,
Jason-JH.Lin

> 
> Reviewed-by: CK Hu <ck.hu@mediatek.com>
>
AngeloGioacchino Del Regno Sept. 26, 2024, 9:50 a.m. UTC | #3 
Il 26/09/24 11:49, Jason-JH Lin (林睿祥) ha scritto:
> On Thu, 2024-09-26 at 09:30 +0000, CK Hu (胡俊光) wrote:
>> Hi, Jason:
>>
>> On Thu, 2024-09-26 at 16:35 +0800, Jason-JH.Lin wrote:
>>> OVL_CON_CLRFMT_MAN is a 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 previous SoCs did not support OVL_CON_CLRFMT_MAN, this means
>>> that the SoC does not support the premultiplied color format.
>>> It will break the original color format setting of MT8173.
>>>
>>> Therefore, the blend_modes is added to the driver data and then
>>> mtk_ovl_fmt_convert() will check the blend_modes to see if
>>> premultiplied supported in current platform.
>>> If it is not supported, use coverage mode to set it to the
>>> supported
>>> color formats to solve the degradation problem.
>>>
>>> Fixes: a3f7f7ef4bfe ("drm/mediatek: Support "Pre-multiplied"
>>> blending in OVL")
>>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
>>> Tested-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
>>> Reviewed-by: AngeloGioacchino Del Regno <
>>> angelogioacchino.delregno@collabora.com>
>>
>> This version has difference over 50% with previous version.
>> It's better to drop these tested-by and reviewed-by tag.
> 
> OK, I'll drop this.
> 

You can keep my R-b tag on this one, I checked it.

Cheers,
Angelo

>>
>>> ---
>>>   drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 41 ++++++++++++++++++++-
>>> ----
>>>   1 file changed, 34 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>>> b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>>> index 89b439dcf3a6..0cf7b80f612e 100644
>>> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>>> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
>>> @@ -146,6 +146,7 @@ struct mtk_disp_ovl_data {
>>>   	bool fmt_rgb565_is_0;
>>>   	bool smi_id_en;
>>>   	bool supports_afbc;
>>> +	const u32 blend_modes;
>>>   	const u32 *formats;
>>>   	size_t num_formats;
>>>   	bool supports_clrfmt_ext;
>>> @@ -386,14 +387,23 @@ void mtk_ovl_layer_off(struct device *dev,
>>> unsigned int idx,
>>>   		      DISP_REG_OVL_RDMA_CTRL(idx));
>>>   }
>>>   
>>> -static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl,
>>> unsigned int fmt,
>>> -				    unsigned int blend_mode)
>>> +static unsigned int mtk_ovl_fmt_convert(struct mtk_disp_ovl *ovl,
>>> +					struct mtk_plane_state *state)
>>>   {
>>> -	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
>>> -	 * is defined in mediatek HW data sheet.
>>> -	 * The alphabet order in XXX is no relation to data
>>> -	 * arrangement in memory.
>>
>> I don't know why you drop these comment.
>> Without this modification,
> 
> Since we change the MACRO to align this to DRM input format order, this
> comment is no longer needed.
> 
> I'll send another patch for this.
> Thanks for such a quick reply!
> 
> Regards,
> Jason-JH.Lin
> 
>>
>> Reviewed-by: CK Hu <ck.hu@mediatek.com>
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 89b439dcf3a6..0cf7b80f612e 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -146,6 +146,7 @@  struct mtk_disp_ovl_data {
 	bool fmt_rgb565_is_0;
 	bool smi_id_en;
 	bool supports_afbc;
+	const u32 blend_modes;
 	const u32 *formats;
 	size_t num_formats;
 	bool supports_clrfmt_ext;
@@ -386,14 +387,23 @@  void mtk_ovl_layer_off(struct device *dev, unsigned int idx,
 		      DISP_REG_OVL_RDMA_CTRL(idx));
 }
 
-static unsigned int ovl_fmt_convert(struct mtk_disp_ovl *ovl, unsigned int fmt,
-				    unsigned int blend_mode)
+static unsigned int mtk_ovl_fmt_convert(struct mtk_disp_ovl *ovl,
+					struct mtk_plane_state *state)
 {
-	/* The return value in switch "MEM_MODE_INPUT_FORMAT_XXX"
-	 * is defined in mediatek HW data sheet.
-	 * The alphabet order in XXX is no relation to data
-	 * arrangement in memory.
+	unsigned int fmt = state->pending.format;
+	unsigned int blend_mode = state->base.pixel_blend_mode;
+
+	/*
+	 * For the platforms where OVL_CON_CLRFMT_MAN is defined in the hardware data sheet
+	 * and supports premultiplied color formats, such as OVL_CON_CLRFMT_PARGB8888.
+	 *
+	 * Check blend_modes in the driver data to see if premultiplied mode is supported.
+	 * If not, use coverage mode instead to set it to the supported color formats.
 	 */
+	if (!(ovl->data->blend_modes & BIT(DRM_MODE_BLEND_PREMULTI)) &&
+	    blend_mode == DRM_MODE_BLEND_PREMULTI)
+		blend_mode = DRM_MODE_BLEND_COVERAGE;
+
 	switch (fmt) {
 	default:
 	case DRM_FORMAT_RGB565:
@@ -471,7 +481,7 @@  void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
 		return;
 	}
 
-	con = ovl_fmt_convert(ovl, fmt, blend_mode);
+	con = mtk_ovl_fmt_convert(ovl, state);
 	if (state->base.fb) {
 		con |= OVL_CON_AEN;
 		con |= state->base.alpha & OVL_CON_ALPHA;
@@ -626,6 +636,8 @@  static const struct mtk_disp_ovl_data mt2701_ovl_driver_data = {
 	.gmc_bits = 8,
 	.layer_nr = 4,
 	.fmt_rgb565_is_0 = false,
+	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
+		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
 	.formats = mt8173_formats,
 	.num_formats = ARRAY_SIZE(mt8173_formats),
 };
@@ -635,6 +647,8 @@  static const struct mtk_disp_ovl_data mt8173_ovl_driver_data = {
 	.gmc_bits = 8,
 	.layer_nr = 4,
 	.fmt_rgb565_is_0 = true,
+	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
+		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
 	.formats = mt8173_formats,
 	.num_formats = ARRAY_SIZE(mt8173_formats),
 };
@@ -644,6 +658,8 @@  static const struct mtk_disp_ovl_data mt8183_ovl_driver_data = {
 	.gmc_bits = 10,
 	.layer_nr = 4,
 	.fmt_rgb565_is_0 = true,
+	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
+		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
 	.formats = mt8173_formats,
 	.num_formats = ARRAY_SIZE(mt8173_formats),
 };
@@ -653,6 +669,8 @@  static const struct mtk_disp_ovl_data mt8183_ovl_2l_driver_data = {
 	.gmc_bits = 10,
 	.layer_nr = 2,
 	.fmt_rgb565_is_0 = true,
+	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
+		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
 	.formats = mt8173_formats,
 	.num_formats = ARRAY_SIZE(mt8173_formats),
 };
@@ -663,6 +681,9 @@  static const struct mtk_disp_ovl_data mt8192_ovl_driver_data = {
 	.layer_nr = 4,
 	.fmt_rgb565_is_0 = true,
 	.smi_id_en = true,
+	.blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
+		       BIT(DRM_MODE_BLEND_COVERAGE) |
+		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
 	.formats = mt8173_formats,
 	.num_formats = ARRAY_SIZE(mt8173_formats),
 };
@@ -673,6 +694,9 @@  static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
 	.layer_nr = 2,
 	.fmt_rgb565_is_0 = true,
 	.smi_id_en = true,
+	.blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
+		       BIT(DRM_MODE_BLEND_COVERAGE) |
+		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
 	.formats = mt8173_formats,
 	.num_formats = ARRAY_SIZE(mt8173_formats),
 };
@@ -684,6 +708,9 @@  static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
 	.fmt_rgb565_is_0 = true,
 	.smi_id_en = true,
 	.supports_afbc = true,
+	.blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
+		       BIT(DRM_MODE_BLEND_COVERAGE) |
+		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
 	.formats = mt8195_formats,
 	.num_formats = ARRAY_SIZE(mt8195_formats),
 	.supports_clrfmt_ext = true,