> ...so that a per-plane logic in mtk_drm_plane can be easily added, because...
I think my concern is more that if we need to validate the format and
the modifier differently because of the plane data, then this method
would provide limited value. For example, on my MT8195, plane ID 38
supports AR30 and BA30, but plane ID 50 does not support any 10-bit
pixel formats.
On Thu, Jul 27, 2023 at 5:37 AM AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:
>
> Il 26/07/23 21:44, Justin Green ha scritto:
> >> Would it make more sense to commmonize function mtk_plane_format_mod_supported()
> >> and call that one here instead?
> > I had considered that, but mtk_plane_format_mod_supported() is
> > required to take a drm_plane as a parameter in order to conform to the
> > type signature defined in drm_plane_funcs, but
> > mtk_drm_mode_fb_create() does not have a drm_plane to provide, since
> > the framebuffer is created later in the function. Technically we don't
> > actually use the drm_plane in the implementation of
> > mtk_plane_format_mod_supported() today, so we could just use a null
> > pointer, but I figured we may one day need to add per-plane logic.
> >
>
> My suggestion was not to use that function as-is, but rather to add a helper like
>
> bool mtk_format_modifier_supported(u32 format, u32 modifier) { ... }
>
> ...so that a per-plane logic in mtk_drm_plane can be easily added, because...
>
> static bool mtk_plane_format_mod_supported(struct drm_plane *plane,
> u32 format, u32 modifier)
> {
> return mtk_format_modifier_supported(format, modifier);
> }
>
> so apart from that, is there any other reason to not do that? :-)
>
> Regards,
> Angelo
>
> >> This is not DRM_FORMAT_MOD_INVALID. Please either explicitly compare against INVALID if that's what you meant, or against LINEAR if that's what you meant, or both.
> > Ack, I meant to use LINEAR. Will update for the next version of the patch.
>
>
