diff mbox series

[v14,1/2] drm: Add connector property to limit max bpc

Message ID 20181012184233.29250-1-radhakrishna.sripada@intel.com (mailing list archive)
State New, archived
Headers show
Series [v14,1/2] drm: Add connector property to limit max bpc | expand

Commit Message

Sripada, Radhakrishna Oct. 12, 2018, 6:42 p.m. UTC
At times 12bpc HDMI cannot be driven due to faulty cables, dongles
level shifters etc. To workaround them we may need to drive the output
at a lower bpc. Currently the user space does not have a way to limit
the bpc. The default bpc to be programmed is decided by the driver and
is run against connector limitations.

Creating a new connector property "max bpc" in order to limit the bpc.
xrandr can make use of this connector property to make sure that bpc does
not exceed the configured value. This property can be used by userspace to
set the bpc.

V2: Initialize max_bpc to satisfy kms_properties
V3: Move the property to drm_connector
V4: Split drm and i915 components(Ville)
V5: Make the property per connector(Ville)
V6: Compare the requested bpc to connector bpc(Daniel)
    Move the attach_property function to core(Ville)
V7: Fix checkpatch warnings
V8: Simplify the connector check code(Ville)
V9: Const display_info(Ville)
V10,V11: Fix CI issues.
V12: Add the Kernel documentation(Daniel)
V14: Crossreference the function name in the doc(Daniel)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Sunpeng Li <sunpeng.li@amd.com>
Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/drm_atomic.c        |  5 +++++
 drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
 drivers/gpu/drm/drm_atomic_uapi.c   |  4 ++++
 drivers/gpu/drm/drm_connector.c     | 41 +++++++++++++++++++++++++++++++++++++
 include/drm/drm_connector.h         | 20 ++++++++++++++++++
 5 files changed, 74 insertions(+)

Comments

Rodrigo Vivi Oct. 24, 2018, 10:49 p.m. UTC | #1
On Fri, Oct 12, 2018 at 11:42:32AM -0700, Radhakrishna Sripada wrote:
> At times 12bpc HDMI cannot be driven due to faulty cables, dongles
> level shifters etc. To workaround them we may need to drive the output
> at a lower bpc. Currently the user space does not have a way to limit
> the bpc. The default bpc to be programmed is decided by the driver and
> is run against connector limitations.
> 
> Creating a new connector property "max bpc" in order to limit the bpc.
> xrandr can make use of this connector property to make sure that bpc does
> not exceed the configured value. This property can be used by userspace to
> set the bpc.
> 
> V2: Initialize max_bpc to satisfy kms_properties
> V3: Move the property to drm_connector
> V4: Split drm and i915 components(Ville)
> V5: Make the property per connector(Ville)
> V6: Compare the requested bpc to connector bpc(Daniel)
>     Move the attach_property function to core(Ville)
> V7: Fix checkpatch warnings
> V8: Simplify the connector check code(Ville)
> V9: Const display_info(Ville)
> V10,V11: Fix CI issues.
> V12: Add the Kernel documentation(Daniel)
> V14: Crossreference the function name in the doc(Daniel)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> Cc: Sunpeng Li <sunpeng.li@amd.com>
> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

This patch looks right to me:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

but we probably need to hold on merge while we wait clarifications
and final rv-b on igt tests....

> ---
>  drivers/gpu/drm/drm_atomic.c        |  5 +++++
>  drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>  drivers/gpu/drm/drm_atomic_uapi.c   |  4 ++++
>  drivers/gpu/drm/drm_connector.c     | 41 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h         | 20 ++++++++++++++++++
>  5 files changed, 74 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 2870ae205237..cd8362dc4f74 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -390,6 +390,11 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
>  {
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_writeback_job *writeback_job = state->writeback_job;
> +	const struct drm_display_info *info = &connector->display_info;
> +
> +	state->max_bpc = info->bpc ? info->bpc : 8;
> +	if (connector->max_bpc_property)
> +		state->max_bpc = min(state->max_bpc, state->max_requested_bpc);
>  
>  	if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job)
>  		return 0;
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 6f66777dca4b..d61a74b254f5 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -650,6 +650,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>  			if (old_connector_state->link_status !=
>  			    new_connector_state->link_status)
>  				new_crtc_state->connectors_changed = true;
> +
> +			if (old_connector_state->max_requested_bpc !=
> +			    new_connector_state->max_requested_bpc)
> +				new_crtc_state->connectors_changed = true;
>  		}
>  
>  		if (funcs->atomic_check)
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index a22d6f269b07..7492819c81ce 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -761,6 +761,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>  
>  		return set_out_fence_for_connector(state->state, connector,
>  						   fence_ptr);
> +	} else if (property == connector->max_bpc_property) {
> +		state->max_requested_bpc = val;
>  	} else if (connector->funcs->atomic_set_property) {
>  		return connector->funcs->atomic_set_property(connector,
>  				state, property, val);
> @@ -825,6 +827,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>  		*val = 0;
>  	} else if (property == config->writeback_out_fence_ptr_property) {
>  		*val = 0;
> +	} else if (property == connector->max_bpc_property) {
> +		*val = state->max_requested_bpc;
>  	} else if (connector->funcs->atomic_get_property) {
>  		return connector->funcs->atomic_get_property(connector,
>  				state, property, val);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 5d01414ec9f7..bdaa29e530d4 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -932,6 +932,13 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
>   *	  is no longer protected and userspace should take appropriate action
>   *	  (whatever that might be).
>   *
> + * max bpc:
> + *	This range property is used by userspace to limit the bit depth. When
> + *	used the driver would limit the bpc in accordance with the valid range
> + *	supported by the hardware and sink. Drivers to use the function
> + *	drm_connector_attach_max_bpc_property() to create and attach the
> + *	property to the connector during initialization.
> + *
>   * Connectors also have one standardized atomic property:
>   *
>   * CRTC_ID:
> @@ -1600,6 +1607,40 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>  EXPORT_SYMBOL(drm_connector_set_link_status_property);
>  
>  /**
> + * drm_connector_attach_max_bpc_property - attach "max bpc" property
> + * @connector: connector to attach max bpc property on.
> + * @min: The minimum bit depth supported by the connector.
> + * @max: The maximum bit depth supported by the connector.
> + *
> + * This is used to add support for limiting the bit depth on a connector.
> + *
> + * Returns:
> + * Zero on success, negative errno on failure.
> + */
> +int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
> +					  int min, int max)
> +{
> +	struct drm_device *dev = connector->dev;
> +	struct drm_property *prop;
> +
> +	prop = connector->max_bpc_property;
> +	if (!prop) {
> +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
> +		if (!prop)
> +			return -ENOMEM;
> +
> +		connector->max_bpc_property = prop;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop, max);
> +	connector->state->max_requested_bpc = max;
> +	connector->state->max_bpc = max;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
> +
> +/**
>   * drm_connector_init_panel_orientation_property -
>   *	initialize the connecters panel_orientation property
>   * @connector: connector for which to init the panel-orientation property.
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 5b3cf909fd5e..a402d16427a1 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -461,6 +461,18 @@ struct drm_connector_state {
>  	 * drm_writeback_signal_completion()
>  	 */
>  	struct drm_writeback_job *writeback_job;
> +
> +	/**
> +	 * @max_requested_bpc: Connector property to limit the maximum bit
> +	 * depth of the pixels.
> +	 */
> +	u8 max_requested_bpc;
> +
> +	/**
> +	 * @max_bpc: Connector max_bpc based on the requested max_bpc property
> +	 * and the connector bpc limitations obtained from edid.
> +	 */
> +	u8 max_bpc;
>  };
>  
>  /**
> @@ -924,6 +936,12 @@ struct drm_connector {
>  	 */
>  	struct drm_property_blob *path_blob_ptr;
>  
> +	/**
> +	 * @max_bpc_property: Default connector property for the max bpc to be
> +	 * driven out of the connector.
> +	 */
> +	struct drm_property *max_bpc_property;
> +
>  #define DRM_CONNECTOR_POLL_HPD (1 << 0)
>  #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
>  #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> @@ -1202,6 +1220,8 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>  					    uint64_t link_status);
>  int drm_connector_init_panel_orientation_property(
>  	struct drm_connector *connector, int width, int height);
> +int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
> +					  int min, int max);
>  
>  /**
>   * struct drm_tile_group - Tile group metadata
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Kazlauskas, Nicholas Nov. 7, 2018, 4:56 p.m. UTC | #2
On 10/24/18 6:49 PM, Rodrigo Vivi wrote:
> On Fri, Oct 12, 2018 at 11:42:32AM -0700, Radhakrishna Sripada wrote:
>> At times 12bpc HDMI cannot be driven due to faulty cables, dongles
>> level shifters etc. To workaround them we may need to drive the output
>> at a lower bpc. Currently the user space does not have a way to limit
>> the bpc. The default bpc to be programmed is decided by the driver and
>> is run against connector limitations.
>>
>> Creating a new connector property "max bpc" in order to limit the bpc.
>> xrandr can make use of this connector property to make sure that bpc does
>> not exceed the configured value. This property can be used by userspace to
>> set the bpc.
>>
>> V2: Initialize max_bpc to satisfy kms_properties
>> V3: Move the property to drm_connector
>> V4: Split drm and i915 components(Ville)
>> V5: Make the property per connector(Ville)
>> V6: Compare the requested bpc to connector bpc(Daniel)
>>      Move the attach_property function to core(Ville)
>> V7: Fix checkpatch warnings
>> V8: Simplify the connector check code(Ville)
>> V9: Const display_info(Ville)
>> V10,V11: Fix CI issues.
>> V12: Add the Kernel documentation(Daniel)
>> V14: Crossreference the function name in the doc(Daniel)
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>> Cc: Sunpeng Li <sunpeng.li@amd.com>
>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Looks good to me. We're seeing similar issues with > 8bpc that this 
would help with.

> 
> This patch looks right to me:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> but we probably need to hold on merge while we wait clarifications
> and final rv-b on igt tests....

Any updates on the igt tests?

> 
>> ---
>>   drivers/gpu/drm/drm_atomic.c        |  5 +++++
>>   drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>>   drivers/gpu/drm/drm_atomic_uapi.c   |  4 ++++
>>   drivers/gpu/drm/drm_connector.c     | 41 +++++++++++++++++++++++++++++++++++++
>>   include/drm/drm_connector.h         | 20 ++++++++++++++++++
>>   5 files changed, 74 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>> index 2870ae205237..cd8362dc4f74 100644
>> --- a/drivers/gpu/drm/drm_atomic.c
>> +++ b/drivers/gpu/drm/drm_atomic.c
>> @@ -390,6 +390,11 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
>>   {
>>   	struct drm_crtc_state *crtc_state;
>>   	struct drm_writeback_job *writeback_job = state->writeback_job;
>> +	const struct drm_display_info *info = &connector->display_info;
>> +
>> +	state->max_bpc = info->bpc ? info->bpc : 8;
>> +	if (connector->max_bpc_property)
>> +		state->max_bpc = min(state->max_bpc, state->max_requested_bpc);
>>   
>>   	if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job)
>>   		return 0;
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>> index 6f66777dca4b..d61a74b254f5 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -650,6 +650,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>>   			if (old_connector_state->link_status !=
>>   			    new_connector_state->link_status)
>>   				new_crtc_state->connectors_changed = true;
>> +
>> +			if (old_connector_state->max_requested_bpc !=
>> +			    new_connector_state->max_requested_bpc)
>> +				new_crtc_state->connectors_changed = true;
>>   		}
>>   
>>   		if (funcs->atomic_check)
>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>> index a22d6f269b07..7492819c81ce 100644
>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>> @@ -761,6 +761,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>>   
>>   		return set_out_fence_for_connector(state->state, connector,
>>   						   fence_ptr);
>> +	} else if (property == connector->max_bpc_property) {
>> +		state->max_requested_bpc = val;
>>   	} else if (connector->funcs->atomic_set_property) {
>>   		return connector->funcs->atomic_set_property(connector,
>>   				state, property, val);
>> @@ -825,6 +827,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>>   		*val = 0;
>>   	} else if (property == config->writeback_out_fence_ptr_property) {
>>   		*val = 0;
>> +	} else if (property == connector->max_bpc_property) {
>> +		*val = state->max_requested_bpc;
>>   	} else if (connector->funcs->atomic_get_property) {
>>   		return connector->funcs->atomic_get_property(connector,
>>   				state, property, val);
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index 5d01414ec9f7..bdaa29e530d4 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -932,6 +932,13 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
>>    *	  is no longer protected and userspace should take appropriate action
>>    *	  (whatever that might be).
>>    *
>> + * max bpc:
>> + *	This range property is used by userspace to limit the bit depth. When
>> + *	used the driver would limit the bpc in accordance with the valid range
>> + *	supported by the hardware and sink. Drivers to use the function
>> + *	drm_connector_attach_max_bpc_property() to create and attach the
>> + *	property to the connector during initialization.
>> + *
>>    * Connectors also have one standardized atomic property:
>>    *
>>    * CRTC_ID:
>> @@ -1600,6 +1607,40 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>>   EXPORT_SYMBOL(drm_connector_set_link_status_property);
>>   
>>   /**
>> + * drm_connector_attach_max_bpc_property - attach "max bpc" property
>> + * @connector: connector to attach max bpc property on.
>> + * @min: The minimum bit depth supported by the connector.
>> + * @max: The maximum bit depth supported by the connector.
>> + *
>> + * This is used to add support for limiting the bit depth on a connector.
>> + *
>> + * Returns:
>> + * Zero on success, negative errno on failure.
>> + */
>> +int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>> +					  int min, int max)
>> +{
>> +	struct drm_device *dev = connector->dev;
>> +	struct drm_property *prop;
>> +
>> +	prop = connector->max_bpc_property;
>> +	if (!prop) {
>> +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
>> +		if (!prop)
>> +			return -ENOMEM;
>> +
>> +		connector->max_bpc_property = prop;
>> +	}
>> +
>> +	drm_object_attach_property(&connector->base, prop, max);
>> +	connector->state->max_requested_bpc = max;
>> +	connector->state->max_bpc = max;
>> +
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
>> +
>> +/**
>>    * drm_connector_init_panel_orientation_property -
>>    *	initialize the connecters panel_orientation property
>>    * @connector: connector for which to init the panel-orientation property.
>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> index 5b3cf909fd5e..a402d16427a1 100644
>> --- a/include/drm/drm_connector.h
>> +++ b/include/drm/drm_connector.h
>> @@ -461,6 +461,18 @@ struct drm_connector_state {
>>   	 * drm_writeback_signal_completion()
>>   	 */
>>   	struct drm_writeback_job *writeback_job;
>> +
>> +	/**
>> +	 * @max_requested_bpc: Connector property to limit the maximum bit
>> +	 * depth of the pixels.
>> +	 */
>> +	u8 max_requested_bpc;
>> +
>> +	/**
>> +	 * @max_bpc: Connector max_bpc based on the requested max_bpc property
>> +	 * and the connector bpc limitations obtained from edid.
>> +	 */
>> +	u8 max_bpc;
>>   };
>>   
>>   /**
>> @@ -924,6 +936,12 @@ struct drm_connector {
>>   	 */
>>   	struct drm_property_blob *path_blob_ptr;
>>   
>> +	/**
>> +	 * @max_bpc_property: Default connector property for the max bpc to be
>> +	 * driven out of the connector.
>> +	 */
>> +	struct drm_property *max_bpc_property;
>> +
>>   #define DRM_CONNECTOR_POLL_HPD (1 << 0)
>>   #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
>>   #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
>> @@ -1202,6 +1220,8 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>>   					    uint64_t link_status);
>>   int drm_connector_init_panel_orientation_property(
>>   	struct drm_connector *connector, int width, int height);
>> +int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>> +					  int min, int max);
>>   
>>   /**
>>    * struct drm_tile_group - Tile group metadata
>> -- 
>> 2.9.3
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Nicholas Kazlauskas
Rodrigo Vivi Nov. 7, 2018, 6:10 p.m. UTC | #3
On Wed, Nov 07, 2018 at 04:56:00PM +0000, Kazlauskas, Nicholas wrote:
> On 10/24/18 6:49 PM, Rodrigo Vivi wrote:
> > On Fri, Oct 12, 2018 at 11:42:32AM -0700, Radhakrishna Sripada wrote:
> >> At times 12bpc HDMI cannot be driven due to faulty cables, dongles
> >> level shifters etc. To workaround them we may need to drive the output
> >> at a lower bpc. Currently the user space does not have a way to limit
> >> the bpc. The default bpc to be programmed is decided by the driver and
> >> is run against connector limitations.
> >>
> >> Creating a new connector property "max bpc" in order to limit the bpc.
> >> xrandr can make use of this connector property to make sure that bpc does
> >> not exceed the configured value. This property can be used by userspace to
> >> set the bpc.
> >>
> >> V2: Initialize max_bpc to satisfy kms_properties
> >> V3: Move the property to drm_connector
> >> V4: Split drm and i915 components(Ville)
> >> V5: Make the property per connector(Ville)
> >> V6: Compare the requested bpc to connector bpc(Daniel)
> >>      Move the attach_property function to core(Ville)
> >> V7: Fix checkpatch warnings
> >> V8: Simplify the connector check code(Ville)
> >> V9: Const display_info(Ville)
> >> V10,V11: Fix CI issues.
> >> V12: Add the Kernel documentation(Daniel)
> >> V14: Crossreference the function name in the doc(Daniel)
> >>
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Cc: Manasi Navare <manasi.d.navare@intel.com>
> >> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
> >> Cc: Sunpeng Li <sunpeng.li@amd.com>
> >> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> 
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Thanks. Although I had pushed through drm-intel-next-queued (dinq) already.

> 
> Looks good to me. We're seeing similar issues with > 8bpc that this 
> would help with.

hm... I hope that pushing to dinq doesn't cause any trouble for you :(

> 
> > 
> > This patch looks right to me:
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > but we probably need to hold on merge while we wait clarifications
> > and final rv-b on igt tests....
> 
> Any updates on the igt tests?

yeap, RK added the property tests. got reviewed and
merged to igt already as well

Thanks,
Rodrigo.

> 
> > 
> >> ---
> >>   drivers/gpu/drm/drm_atomic.c        |  5 +++++
> >>   drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
> >>   drivers/gpu/drm/drm_atomic_uapi.c   |  4 ++++
> >>   drivers/gpu/drm/drm_connector.c     | 41 +++++++++++++++++++++++++++++++++++++
> >>   include/drm/drm_connector.h         | 20 ++++++++++++++++++
> >>   5 files changed, 74 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> >> index 2870ae205237..cd8362dc4f74 100644
> >> --- a/drivers/gpu/drm/drm_atomic.c
> >> +++ b/drivers/gpu/drm/drm_atomic.c
> >> @@ -390,6 +390,11 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
> >>   {
> >>   	struct drm_crtc_state *crtc_state;
> >>   	struct drm_writeback_job *writeback_job = state->writeback_job;
> >> +	const struct drm_display_info *info = &connector->display_info;
> >> +
> >> +	state->max_bpc = info->bpc ? info->bpc : 8;
> >> +	if (connector->max_bpc_property)
> >> +		state->max_bpc = min(state->max_bpc, state->max_requested_bpc);
> >>   
> >>   	if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job)
> >>   		return 0;
> >> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> >> index 6f66777dca4b..d61a74b254f5 100644
> >> --- a/drivers/gpu/drm/drm_atomic_helper.c
> >> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> >> @@ -650,6 +650,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> >>   			if (old_connector_state->link_status !=
> >>   			    new_connector_state->link_status)
> >>   				new_crtc_state->connectors_changed = true;
> >> +
> >> +			if (old_connector_state->max_requested_bpc !=
> >> +			    new_connector_state->max_requested_bpc)
> >> +				new_crtc_state->connectors_changed = true;
> >>   		}
> >>   
> >>   		if (funcs->atomic_check)
> >> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> >> index a22d6f269b07..7492819c81ce 100644
> >> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> >> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> >> @@ -761,6 +761,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
> >>   
> >>   		return set_out_fence_for_connector(state->state, connector,
> >>   						   fence_ptr);
> >> +	} else if (property == connector->max_bpc_property) {
> >> +		state->max_requested_bpc = val;
> >>   	} else if (connector->funcs->atomic_set_property) {
> >>   		return connector->funcs->atomic_set_property(connector,
> >>   				state, property, val);
> >> @@ -825,6 +827,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
> >>   		*val = 0;
> >>   	} else if (property == config->writeback_out_fence_ptr_property) {
> >>   		*val = 0;
> >> +	} else if (property == connector->max_bpc_property) {
> >> +		*val = state->max_requested_bpc;
> >>   	} else if (connector->funcs->atomic_get_property) {
> >>   		return connector->funcs->atomic_get_property(connector,
> >>   				state, property, val);
> >> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> >> index 5d01414ec9f7..bdaa29e530d4 100644
> >> --- a/drivers/gpu/drm/drm_connector.c
> >> +++ b/drivers/gpu/drm/drm_connector.c
> >> @@ -932,6 +932,13 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
> >>    *	  is no longer protected and userspace should take appropriate action
> >>    *	  (whatever that might be).
> >>    *
> >> + * max bpc:
> >> + *	This range property is used by userspace to limit the bit depth. When
> >> + *	used the driver would limit the bpc in accordance with the valid range
> >> + *	supported by the hardware and sink. Drivers to use the function
> >> + *	drm_connector_attach_max_bpc_property() to create and attach the
> >> + *	property to the connector during initialization.
> >> + *
> >>    * Connectors also have one standardized atomic property:
> >>    *
> >>    * CRTC_ID:
> >> @@ -1600,6 +1607,40 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
> >>   EXPORT_SYMBOL(drm_connector_set_link_status_property);
> >>   
> >>   /**
> >> + * drm_connector_attach_max_bpc_property - attach "max bpc" property
> >> + * @connector: connector to attach max bpc property on.
> >> + * @min: The minimum bit depth supported by the connector.
> >> + * @max: The maximum bit depth supported by the connector.
> >> + *
> >> + * This is used to add support for limiting the bit depth on a connector.
> >> + *
> >> + * Returns:
> >> + * Zero on success, negative errno on failure.
> >> + */
> >> +int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
> >> +					  int min, int max)
> >> +{
> >> +	struct drm_device *dev = connector->dev;
> >> +	struct drm_property *prop;
> >> +
> >> +	prop = connector->max_bpc_property;
> >> +	if (!prop) {
> >> +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
> >> +		if (!prop)
> >> +			return -ENOMEM;
> >> +
> >> +		connector->max_bpc_property = prop;
> >> +	}
> >> +
> >> +	drm_object_attach_property(&connector->base, prop, max);
> >> +	connector->state->max_requested_bpc = max;
> >> +	connector->state->max_bpc = max;
> >> +
> >> +	return 0;
> >> +}
> >> +EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
> >> +
> >> +/**
> >>    * drm_connector_init_panel_orientation_property -
> >>    *	initialize the connecters panel_orientation property
> >>    * @connector: connector for which to init the panel-orientation property.
> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> >> index 5b3cf909fd5e..a402d16427a1 100644
> >> --- a/include/drm/drm_connector.h
> >> +++ b/include/drm/drm_connector.h
> >> @@ -461,6 +461,18 @@ struct drm_connector_state {
> >>   	 * drm_writeback_signal_completion()
> >>   	 */
> >>   	struct drm_writeback_job *writeback_job;
> >> +
> >> +	/**
> >> +	 * @max_requested_bpc: Connector property to limit the maximum bit
> >> +	 * depth of the pixels.
> >> +	 */
> >> +	u8 max_requested_bpc;
> >> +
> >> +	/**
> >> +	 * @max_bpc: Connector max_bpc based on the requested max_bpc property
> >> +	 * and the connector bpc limitations obtained from edid.
> >> +	 */
> >> +	u8 max_bpc;
> >>   };
> >>   
> >>   /**
> >> @@ -924,6 +936,12 @@ struct drm_connector {
> >>   	 */
> >>   	struct drm_property_blob *path_blob_ptr;
> >>   
> >> +	/**
> >> +	 * @max_bpc_property: Default connector property for the max bpc to be
> >> +	 * driven out of the connector.
> >> +	 */
> >> +	struct drm_property *max_bpc_property;
> >> +
> >>   #define DRM_CONNECTOR_POLL_HPD (1 << 0)
> >>   #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
> >>   #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
> >> @@ -1202,6 +1220,8 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
> >>   					    uint64_t link_status);
> >>   int drm_connector_init_panel_orientation_property(
> >>   	struct drm_connector *connector, int width, int height);
> >> +int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
> >> +					  int min, int max);
> >>   
> >>   /**
> >>    * struct drm_tile_group - Tile group metadata
> >> -- 
> >> 2.9.3
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> Nicholas Kazlauskas
Kazlauskas, Nicholas Nov. 7, 2018, 6:15 p.m. UTC | #4
On 11/7/18 1:10 PM, Rodrigo Vivi wrote:
> On Wed, Nov 07, 2018 at 04:56:00PM +0000, Kazlauskas, Nicholas wrote:
>> On 10/24/18 6:49 PM, Rodrigo Vivi wrote:
>>> On Fri, Oct 12, 2018 at 11:42:32AM -0700, Radhakrishna Sripada wrote:
>>>> At times 12bpc HDMI cannot be driven due to faulty cables, dongles
>>>> level shifters etc. To workaround them we may need to drive the output
>>>> at a lower bpc. Currently the user space does not have a way to limit
>>>> the bpc. The default bpc to be programmed is decided by the driver and
>>>> is run against connector limitations.
>>>>
>>>> Creating a new connector property "max bpc" in order to limit the bpc.
>>>> xrandr can make use of this connector property to make sure that bpc does
>>>> not exceed the configured value. This property can be used by userspace to
>>>> set the bpc.
>>>>
>>>> V2: Initialize max_bpc to satisfy kms_properties
>>>> V3: Move the property to drm_connector
>>>> V4: Split drm and i915 components(Ville)
>>>> V5: Make the property per connector(Ville)
>>>> V6: Compare the requested bpc to connector bpc(Daniel)
>>>>       Move the attach_property function to core(Ville)
>>>> V7: Fix checkpatch warnings
>>>> V8: Simplify the connector check code(Ville)
>>>> V9: Const display_info(Ville)
>>>> V10,V11: Fix CI issues.
>>>> V12: Add the Kernel documentation(Daniel)
>>>> V14: Crossreference the function name in the doc(Daniel)
>>>>
>>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Cc: Kishore Kadiyala <kishore.kadiyala@intel.com>
>>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>> Cc: Manasi Navare <manasi.d.navare@intel.com>
>>>> Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
>>>> Cc: Sunpeng Li <sunpeng.li@amd.com>
>>>> Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>>
>> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> 
> Thanks. Although I had pushed through drm-intel-next-queued (dinq) already.
> 
>>
>> Looks good to me. We're seeing similar issues with > 8bpc that this
>> would help with.
> 
> hm... I hope that pushing to dinq doesn't cause any trouble for you :(
> 
>>
>>>
>>> This patch looks right to me:
>>>
>>> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>>
>>> but we probably need to hold on merge while we wait clarifications
>>> and final rv-b on igt tests....
>>
>> Any updates on the igt tests?
> 
> yeap, RK added the property tests. got reviewed and
> merged to igt already as well
> 
> Thanks,
> Rodrigo.

Thanks for the update.

I think in the meantime while waiting for this to land in drm we'll make 
use of a driver specific property with the same name and swap it out 
later for the common property.

Nicholas Kazlauskas

> 
>>
>>>
>>>> ---
>>>>    drivers/gpu/drm/drm_atomic.c        |  5 +++++
>>>>    drivers/gpu/drm/drm_atomic_helper.c |  4 ++++
>>>>    drivers/gpu/drm/drm_atomic_uapi.c   |  4 ++++
>>>>    drivers/gpu/drm/drm_connector.c     | 41 +++++++++++++++++++++++++++++++++++++
>>>>    include/drm/drm_connector.h         | 20 ++++++++++++++++++
>>>>    5 files changed, 74 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>>>> index 2870ae205237..cd8362dc4f74 100644
>>>> --- a/drivers/gpu/drm/drm_atomic.c
>>>> +++ b/drivers/gpu/drm/drm_atomic.c
>>>> @@ -390,6 +390,11 @@ static int drm_atomic_connector_check(struct drm_connector *connector,
>>>>    {
>>>>    	struct drm_crtc_state *crtc_state;
>>>>    	struct drm_writeback_job *writeback_job = state->writeback_job;
>>>> +	const struct drm_display_info *info = &connector->display_info;
>>>> +
>>>> +	state->max_bpc = info->bpc ? info->bpc : 8;
>>>> +	if (connector->max_bpc_property)
>>>> +		state->max_bpc = min(state->max_bpc, state->max_requested_bpc);
>>>>    
>>>>    	if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job)
>>>>    		return 0;
>>>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>>>> index 6f66777dca4b..d61a74b254f5 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>>>> @@ -650,6 +650,10 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
>>>>    			if (old_connector_state->link_status !=
>>>>    			    new_connector_state->link_status)
>>>>    				new_crtc_state->connectors_changed = true;
>>>> +
>>>> +			if (old_connector_state->max_requested_bpc !=
>>>> +			    new_connector_state->max_requested_bpc)
>>>> +				new_crtc_state->connectors_changed = true;
>>>>    		}
>>>>    
>>>>    		if (funcs->atomic_check)
>>>> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
>>>> index a22d6f269b07..7492819c81ce 100644
>>>> --- a/drivers/gpu/drm/drm_atomic_uapi.c
>>>> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
>>>> @@ -761,6 +761,8 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector,
>>>>    
>>>>    		return set_out_fence_for_connector(state->state, connector,
>>>>    						   fence_ptr);
>>>> +	} else if (property == connector->max_bpc_property) {
>>>> +		state->max_requested_bpc = val;
>>>>    	} else if (connector->funcs->atomic_set_property) {
>>>>    		return connector->funcs->atomic_set_property(connector,
>>>>    				state, property, val);
>>>> @@ -825,6 +827,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector,
>>>>    		*val = 0;
>>>>    	} else if (property == config->writeback_out_fence_ptr_property) {
>>>>    		*val = 0;
>>>> +	} else if (property == connector->max_bpc_property) {
>>>> +		*val = state->max_requested_bpc;
>>>>    	} else if (connector->funcs->atomic_get_property) {
>>>>    		return connector->funcs->atomic_get_property(connector,
>>>>    				state, property, val);
>>>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>>>> index 5d01414ec9f7..bdaa29e530d4 100644
>>>> --- a/drivers/gpu/drm/drm_connector.c
>>>> +++ b/drivers/gpu/drm/drm_connector.c
>>>> @@ -932,6 +932,13 @@ DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
>>>>     *	  is no longer protected and userspace should take appropriate action
>>>>     *	  (whatever that might be).
>>>>     *
>>>> + * max bpc:
>>>> + *	This range property is used by userspace to limit the bit depth. When
>>>> + *	used the driver would limit the bpc in accordance with the valid range
>>>> + *	supported by the hardware and sink. Drivers to use the function
>>>> + *	drm_connector_attach_max_bpc_property() to create and attach the
>>>> + *	property to the connector during initialization.
>>>> + *
>>>>     * Connectors also have one standardized atomic property:
>>>>     *
>>>>     * CRTC_ID:
>>>> @@ -1600,6 +1607,40 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>>>>    EXPORT_SYMBOL(drm_connector_set_link_status_property);
>>>>    
>>>>    /**
>>>> + * drm_connector_attach_max_bpc_property - attach "max bpc" property
>>>> + * @connector: connector to attach max bpc property on.
>>>> + * @min: The minimum bit depth supported by the connector.
>>>> + * @max: The maximum bit depth supported by the connector.
>>>> + *
>>>> + * This is used to add support for limiting the bit depth on a connector.
>>>> + *
>>>> + * Returns:
>>>> + * Zero on success, negative errno on failure.
>>>> + */
>>>> +int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>>>> +					  int min, int max)
>>>> +{
>>>> +	struct drm_device *dev = connector->dev;
>>>> +	struct drm_property *prop;
>>>> +
>>>> +	prop = connector->max_bpc_property;
>>>> +	if (!prop) {
>>>> +		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
>>>> +		if (!prop)
>>>> +			return -ENOMEM;
>>>> +
>>>> +		connector->max_bpc_property = prop;
>>>> +	}
>>>> +
>>>> +	drm_object_attach_property(&connector->base, prop, max);
>>>> +	connector->state->max_requested_bpc = max;
>>>> +	connector->state->max_bpc = max;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
>>>> +
>>>> +/**
>>>>     * drm_connector_init_panel_orientation_property -
>>>>     *	initialize the connecters panel_orientation property
>>>>     * @connector: connector for which to init the panel-orientation property.
>>>> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>>>> index 5b3cf909fd5e..a402d16427a1 100644
>>>> --- a/include/drm/drm_connector.h
>>>> +++ b/include/drm/drm_connector.h
>>>> @@ -461,6 +461,18 @@ struct drm_connector_state {
>>>>    	 * drm_writeback_signal_completion()
>>>>    	 */
>>>>    	struct drm_writeback_job *writeback_job;
>>>> +
>>>> +	/**
>>>> +	 * @max_requested_bpc: Connector property to limit the maximum bit
>>>> +	 * depth of the pixels.
>>>> +	 */
>>>> +	u8 max_requested_bpc;
>>>> +
>>>> +	/**
>>>> +	 * @max_bpc: Connector max_bpc based on the requested max_bpc property
>>>> +	 * and the connector bpc limitations obtained from edid.
>>>> +	 */
>>>> +	u8 max_bpc;
>>>>    };
>>>>    
>>>>    /**
>>>> @@ -924,6 +936,12 @@ struct drm_connector {
>>>>    	 */
>>>>    	struct drm_property_blob *path_blob_ptr;
>>>>    
>>>> +	/**
>>>> +	 * @max_bpc_property: Default connector property for the max bpc to be
>>>> +	 * driven out of the connector.
>>>> +	 */
>>>> +	struct drm_property *max_bpc_property;
>>>> +
>>>>    #define DRM_CONNECTOR_POLL_HPD (1 << 0)
>>>>    #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
>>>>    #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
>>>> @@ -1202,6 +1220,8 @@ void drm_connector_set_link_status_property(struct drm_connector *connector,
>>>>    					    uint64_t link_status);
>>>>    int drm_connector_init_panel_orientation_property(
>>>>    	struct drm_connector *connector, int width, int height);
>>>> +int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
>>>> +					  int min, int max);
>>>>    
>>>>    /**
>>>>     * struct drm_tile_group - Tile group metadata
>>>> -- 
>>>> 2.9.3
>>>>
>>>> _______________________________________________
>>>> Intel-gfx mailing list
>>>> Intel-gfx@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>> Nicholas Kazlauska
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 2870ae205237..cd8362dc4f74 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -390,6 +390,11 @@  static int drm_atomic_connector_check(struct drm_connector *connector,
 {
 	struct drm_crtc_state *crtc_state;
 	struct drm_writeback_job *writeback_job = state->writeback_job;
+	const struct drm_display_info *info = &connector->display_info;
+
+	state->max_bpc = info->bpc ? info->bpc : 8;
+	if (connector->max_bpc_property)
+		state->max_bpc = min(state->max_bpc, state->max_requested_bpc);
 
 	if ((connector->connector_type != DRM_MODE_CONNECTOR_WRITEBACK) || !writeback_job)
 		return 0;
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 6f66777dca4b..d61a74b254f5 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -650,6 +650,10 @@  drm_atomic_helper_check_modeset(struct drm_device *dev,
 			if (old_connector_state->link_status !=
 			    new_connector_state->link_status)
 				new_crtc_state->connectors_changed = true;
+
+			if (old_connector_state->max_requested_bpc !=
+			    new_connector_state->max_requested_bpc)
+				new_crtc_state->connectors_changed = true;
 		}
 
 		if (funcs->atomic_check)
diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
index a22d6f269b07..7492819c81ce 100644
--- a/drivers/gpu/drm/drm_atomic_uapi.c
+++ b/drivers/gpu/drm/drm_atomic_uapi.c
@@ -761,6 +761,8 @@  static int drm_atomic_connector_set_property(struct drm_connector *connector,
 
 		return set_out_fence_for_connector(state->state, connector,
 						   fence_ptr);
+	} else if (property == connector->max_bpc_property) {
+		state->max_requested_bpc = val;
 	} else if (connector->funcs->atomic_set_property) {
 		return connector->funcs->atomic_set_property(connector,
 				state, property, val);
@@ -825,6 +827,8 @@  drm_atomic_connector_get_property(struct drm_connector *connector,
 		*val = 0;
 	} else if (property == config->writeback_out_fence_ptr_property) {
 		*val = 0;
+	} else if (property == connector->max_bpc_property) {
+		*val = state->max_requested_bpc;
 	} else if (connector->funcs->atomic_get_property) {
 		return connector->funcs->atomic_get_property(connector,
 				state, property, val);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 5d01414ec9f7..bdaa29e530d4 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -932,6 +932,13 @@  DRM_ENUM_NAME_FN(drm_get_content_protection_name, drm_cp_enum_list)
  *	  is no longer protected and userspace should take appropriate action
  *	  (whatever that might be).
  *
+ * max bpc:
+ *	This range property is used by userspace to limit the bit depth. When
+ *	used the driver would limit the bpc in accordance with the valid range
+ *	supported by the hardware and sink. Drivers to use the function
+ *	drm_connector_attach_max_bpc_property() to create and attach the
+ *	property to the connector during initialization.
+ *
  * Connectors also have one standardized atomic property:
  *
  * CRTC_ID:
@@ -1600,6 +1607,40 @@  void drm_connector_set_link_status_property(struct drm_connector *connector,
 EXPORT_SYMBOL(drm_connector_set_link_status_property);
 
 /**
+ * drm_connector_attach_max_bpc_property - attach "max bpc" property
+ * @connector: connector to attach max bpc property on.
+ * @min: The minimum bit depth supported by the connector.
+ * @max: The maximum bit depth supported by the connector.
+ *
+ * This is used to add support for limiting the bit depth on a connector.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
+int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
+					  int min, int max)
+{
+	struct drm_device *dev = connector->dev;
+	struct drm_property *prop;
+
+	prop = connector->max_bpc_property;
+	if (!prop) {
+		prop = drm_property_create_range(dev, 0, "max bpc", min, max);
+		if (!prop)
+			return -ENOMEM;
+
+		connector->max_bpc_property = prop;
+	}
+
+	drm_object_attach_property(&connector->base, prop, max);
+	connector->state->max_requested_bpc = max;
+	connector->state->max_bpc = max;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_attach_max_bpc_property);
+
+/**
  * drm_connector_init_panel_orientation_property -
  *	initialize the connecters panel_orientation property
  * @connector: connector for which to init the panel-orientation property.
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 5b3cf909fd5e..a402d16427a1 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -461,6 +461,18 @@  struct drm_connector_state {
 	 * drm_writeback_signal_completion()
 	 */
 	struct drm_writeback_job *writeback_job;
+
+	/**
+	 * @max_requested_bpc: Connector property to limit the maximum bit
+	 * depth of the pixels.
+	 */
+	u8 max_requested_bpc;
+
+	/**
+	 * @max_bpc: Connector max_bpc based on the requested max_bpc property
+	 * and the connector bpc limitations obtained from edid.
+	 */
+	u8 max_bpc;
 };
 
 /**
@@ -924,6 +936,12 @@  struct drm_connector {
 	 */
 	struct drm_property_blob *path_blob_ptr;
 
+	/**
+	 * @max_bpc_property: Default connector property for the max bpc to be
+	 * driven out of the connector.
+	 */
+	struct drm_property *max_bpc_property;
+
 #define DRM_CONNECTOR_POLL_HPD (1 << 0)
 #define DRM_CONNECTOR_POLL_CONNECT (1 << 1)
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
@@ -1202,6 +1220,8 @@  void drm_connector_set_link_status_property(struct drm_connector *connector,
 					    uint64_t link_status);
 int drm_connector_init_panel_orientation_property(
 	struct drm_connector *connector, int width, int height);
+int drm_connector_attach_max_bpc_property(struct drm_connector *connector,
+					  int min, int max);
 
 /**
  * struct drm_tile_group - Tile group metadata