Re: [PATCH 05/11] drm/mediatek: ovl: Implement support for Inverse Gamma

["CK Hu (胡俊光)" <ck.hu@mediatek.com>]

On Wed, 2026-02-25 at 08:54 +0000, CK Hu (胡俊光) wrote:
> 
> On Tue, 2025-12-23 at 16:44 -0300, Nícolas F. R. A. Prado wrote:
> > External email : Please do not click links or open attachments until you have verified the sender or the content.
> > 
> > 
> > The OVL hardware allows selecting between different fixed transfer
> > functions for each layer through the Inverse Gamma setting. Available
> > functions are scRGB and BT.709. Implement support for it and expose it
> > as a colorop through the DRM plane color pipeline uAPI.
> > 
> > Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> > ---

[snip]

> > +
> > +#define MAX_COLOR_PIPELINE_OPS 3

After review later patch, this is hardware limitation.
The three ops is:
1. Inverse gamme
2. 3x3 matrix
3. gamme

Please add this information.

> 
> Why is 3? Is this hardware limitation?
> Provide some information about this value,
> so we know it's possible to change it or not.
> 
> > +
> > +static int
> > +mtk_ovl_initialize_plane_color_pipeline(struct drm_plane *plane,
> > +                                       struct drm_prop_enum_list *pipeline)
> > +{
> > +       struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS];
> > +       struct drm_device *dev = plane->dev;
> > +       int i = 0;
> > +       int ret;
> > +
> > +       memset(ops, 0, sizeof(ops));
> > +
> > +       /* 1st op: OVL's Inverse Gamma */
> > +       ops[i] = kzalloc(sizeof(*ops[i]), GFP_KERNEL);
> > +       if (!ops[i]) {
> > +               ret = -ENOMEM;
> > +               goto err_alloc;
> > +       }
> > +
> > +       ret = drm_plane_colorop_curve_1d_init(dev, ops[i], plane,
> > +                                             &mtk_ovl_colorop_funcs,
> > +                                             igamma_supported_tfs,
> > +                                             DRM_COLOROP_FLAG_ALLOW_BYPASS);
> 
> If other sub driver (maybe ovl adapter) also has inverse gamma,
> most part of this function looks common and only mtk_ovl_colorop_funcs and igamma_supported_tfs are sub driver specific.
> So move the common part to mtk_crtc.c and sub driver provide function to query mtk_ovl_colorop_funcs and igamma_supported_tfs.
> (mtk_ovl_colorop_funcs looks not a sub driver specific function, maybe it could be common part)

Sorry, I've reviewed the later patch, and I find the whole function is sub driver specific.
So forget this comment.

Regards,
CK

> 
> Regards,
> CK
> 
> > +       if (ret)
> > +               goto err_colorop_init;
> > +
> > +       pipeline->type = ops[0]->base.id;
> > +       pipeline->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", ops[0]->base.id);
> > +
> > +       return 0;
> > +
> > +err_colorop_init:
> > +       kfree(ops[i]);
> > +
> > +err_alloc:
> > +       i--;
> > +       for (; i >= 0; i--) {
> > +               drm_colorop_cleanup(ops[i]);
> > +               kfree(ops[i]);
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > 
> 
> 
> ************* MEDIATEK Confidentiality Notice
>  ********************
> The information contained in this e-mail message (including any 
> attachments) may be confidential, proprietary, privileged, or otherwise
> exempt from disclosure under applicable laws. It is intended to be 
> conveyed only to the designated recipient(s). Any use, dissemination, 
> distribution, printing, retaining or copying of this e-mail (including its 
> attachments) by unintended recipient(s) is strictly prohibited and may 
> be unlawful. If you are not an intended recipient of this e-mail, or believe
>  
> 
> 
> 
> that you have received this e-mail in error, please notify the sender 
> immediately (by replying to this e-mail), delete any and all copies of 
> this e-mail (including any attachments) from your system, and do not
> disclose the content of this e-mail to any other person. Thank you!