[PATCH v2 2/2] drm/connector: automatically set immutable flag for max_bpc property

[Abhinav Kumar]

+ IGT dev

On 6/22/2024 10:40 PM, Dmitry Baryshkov wrote:
> With the introduction of the HDMI Connector framework the driver might
> end up creating the max_bpc property with min = max = 8. IGT insists
> that such properties carry the 'immutable' flag. Automatically set the
> flag if the driver asks for the max_bpc property with min == max.
> 

This change does not look right to me.

I wonder why we need this check because DRM_MODE_PROP_IMMUTABLE means 
that as per the doc, userspace cannot change the property.

          * DRM_MODE_PROP_IMMUTABLE
          *     Set for properties whose values cannot be changed by
          *     userspace. The kernel is allowed to update the value of 
these
          *     properties. This is generally used to expose probe state to
          *     userspace, e.g. the EDID, or the connector path property 
on DP
          *     MST sinks. Kernel can update the value of an immutable 
property
          *     by calling drm_object_property_set_value().
          */

Here we are allowing userspace to change max_bpc


drm_atomic_connector_set_property()
{
	**********

         } else if (property == connector->max_bpc_property) {
                 state->max_requested_bpc = val;

	**********
}

I believe you are referring to this IGT check right?

https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_properties.c#L428

I think we should fix IGT in this case unless there is some reason we 
are missing. Because just because it has the same min and max does not 
mean its immutable by the doc of the IMMUTABLE flag.


> Fixes: aadb3e16b8f3 ("drm/connector: hdmi: Add output BPC to the connector state")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> ---
>   drivers/gpu/drm/drm_connector.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ab6ab7ff7ea8..33847fd63628 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -2610,7 +2610,12 @@ int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>   
>   	prop = connector->max_bpc_property;
>   	if (!prop) {
> -		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
> +		u32 flags = 0;
> +
> +		if (min == max)
> +			flags |= DRM_MODE_PROP_IMMUTABLE;
> +
> +		prop = drm_property_create_range(dev, flags, "max bpc", min, max);
>   		if (!prop)
>   			return -ENOMEM;
>   
>