linux-kernel - Re: [PATCH v5 2/2] drm/mediatek: Add blend_modes to mtk_plane

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bcdabb15b002c47512358cfabdfb49bc12f42596.camel@mediatek.com>
Date: Thu, 26 Sep 2024 02:34:40 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>,
	"alpernebiyasak@...il.com" <alpernebiyasak@...il.com>,
	"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>, "AngeloGioacchino Del
 Regno" <angelogioacchino.delregno@...labora.com>
CC: "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Singo Chang (張興國) <Singo.Chang@...iatek.com>,
	Shawn Sung (宋孝謙) <Shawn.Sung@...iatek.com>,
	Nancy Lin (林欣螢) <Nancy.Lin@...iatek.com>,
	"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
	Project_Global_Chrome_Upstream_Group
	<Project_Global_Chrome_Upstream_Group@...iatek.com>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH v5 2/2] drm/mediatek: Add blend_modes to mtk_plane_init()
 for different SoCs

Hi, Jason:

On Wed, 2024-09-25 at 18:19 +0800, Jason-JH.Lin wrote:
> Since some SoCs support premultiplied pixel formats but some do not,
> the blend_modes parameter is added to mtk_plane_init(), which is
> obtained from the mtk_ddp_comp_get_blend_modes function implemented
> in different OVL components.
> 
> The OVL component can use driver data to set the blend mode
> capabilities for different SoCs.
> 
> Fixes: 4225d5d5e779 ("drm/mediatek: Support alpha blending in display driver")
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> ---

[snip]

> +
>  const u32 *mtk_ovl_get_formats(struct device *dev)
>  {
>  	struct mtk_disp_ovl *ovl = dev_get_drvdata(dev);
> @@ -674,6 +682,8 @@ static const struct mtk_disp_ovl_data mt2701_ovl_driver_data = {
>  	.layer_nr = 4,
>  	.fmt_convert = mtk_ovl_fmt_convert,
>  	.fmt_rgb565_is_0 = false,
> +	.blend_modes = BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),

When blend_modes has no pre-multiplied, fmt_convert is mtk_ovl_fmt_convert().
When blend_modes pre-multiplied, fmt_convert is mtk_ovl_fmt_convert_with_blend().

I think we could keep blend_modes and drop fmt_convert.

if (data->blend_modes & BIT(DRM_MODE_BLEND_PREMULTI))
	mtk_ovl_fmt_convert_with_blend(fmt, blend_modes);
else
	mtk_ovl_fmt_convert(fmt);

In current patch, when SoC does not support pre-multiplied but user space set pre-multiplied buffer,
driver would keep going and treat it as coverage buffer.

I think the result is wrong but it's error handling.
If we accept this error, why not keep only mtk_ovl_fmt_convert_with_blend() and drop mtk_ovl_fmt_convert().

if (data->blend_modes & BIT(DRM_MODE_BLEND_PREMULTI))
	mtk_ovl_fmt_convert_with_blend(fmt, blend_modes);
else
	mtk_ovl_fmt_convert_with_blend(fmt, blend_modes);

and this would result in:

mtk_ovl_fmt_convert_with_blend(fmt, blend_modes);


>  	.formats = mt8173_formats,
>  	.num_formats = ARRAY_SIZE(mt8173_formats),
>  };

[snip]

> @@ -715,6 +731,9 @@ static const struct mtk_disp_ovl_data mt8192_ovl_driver_data = {
>  	.fmt_convert = mtk_ovl_fmt_convert_with_blend,
>  	.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),
>  };

[snip]

> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> index c6768210b08b..2d47b6eb4c19 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl_adaptor.c
> @@ -400,6 +400,13 @@ void mtk_ovl_adaptor_disable_vblank(struct device *dev)
>  	mtk_ethdr_disable_vblank(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_ETHDR0]);
>  }
>  
> +const u32 mtk_ovl_adaptor_get_blend_modes(struct device *dev)
> +{
> +	struct mtk_disp_ovl_adaptor *ovl_adaptor = dev_get_drvdata(dev);
> +
> +	return mtk_mdp_rdma_get_blend_modes(ovl_adaptor->ovl_adaptor_comp[OVL_ADAPTOR_MDP_RDMA0]);

RDMA has no alpha blending function.
Get blend mode from ethdr.

> +}
> +
>  

[snip]

>  
> @@ -366,10 +366,7 @@ int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane,
>  	if (err)
>  		DRM_ERROR("failed to create property: alpha\n");
>  
> -	err = drm_plane_create_blend_mode_property(plane,
> -						   BIT(DRM_MODE_BLEND_PREMULTI) |
> -						   BIT(DRM_MODE_BLEND_COVERAGE) |
> -						   BIT(DRM_MODE_BLEND_PIXEL_NONE));
> +	err = drm_plane_create_blend_mode_property(plane, blend_modes);

For disp_rdma, blend_modes is 0, I this is not necessary to create blend mode property.

Regards,
CK

>  	if (err)
>  		DRM_ERROR("failed to create property: blend_mode\n");
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.h b/drivers/gpu/drm/mediatek/mtk_plane.h
> index 5b177eac67b7..3b13b89989c7 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.h
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.h
> @@ -48,6 +48,6 @@ to_mtk_plane_state(struct drm_plane_state *state)
>  
>  int mtk_plane_init(struct drm_device *dev, struct drm_plane *plane,
>  		   unsigned long possible_crtcs, enum drm_plane_type type,
> -		   unsigned int supported_rotations, const u32 *formats,
> -		   size_t num_formats, unsigned int plane_idx);
> +		   unsigned int supported_rotations, const u32 blend_modes,
> +		   const u32 *formats, size_t num_formats, unsigned int plane_idx);
>  #endif