Re: [PATCH v5 2/2] drm/mediatek: Add blend_modes to mtk_plane_init() for different SoCs

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

On Thu, 2024-09-26 at 02:34 +0000, CK Hu (胡俊光) wrote:
> 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@mediatek.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.

Yes, OVL in previous SoCs only support the color format with coverage
mode, even if user space set pre-multiplied buffer.
I'll write the comment in this part of code.

> 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);

OK I'll drop the function pointer and use blend_modes to cope with
this.

> [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.

OK, I'll change it to 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_P
> > REMULTI) |
> > -						   BIT(DRM_MODE_BLEND_C
> > OVERAGE) |
> > -						   BIT(DRM_MODE_BLEND_P
> > IXEL_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.

OK, I'll add the 0 checking for this.

Regards,
Jason-JH.Lin

> 
> Regards,
> CK