[Intel-gfx] FW: [PATCH] drm/i915: Reject HDMI 12bpc if the sink doesn't indicate support

[Sharma, Shashank]

Regards

Shashank

> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
>
> Check that the sink really declared 12bpc support before we enable it.
> This should not actually never happen since it's mandatory for HDMI sinks to support 12bpc if they support any deep color modes. But reality disagrees with the theory and there are actually sinks in the wild that violate the spec.
>
> v2: Fix the output_types check
>      Update commit message to state that these things are in fact real
>
> Cc: stable at vger.kernel.org
> Cc: Nicholas Sielicki <nicholas.sielicki at gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99250
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_hdmi.c | 24 +++++++++++++++++++++---
>   1 file changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index a580de80d2b5..2bf5915284aa 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1298,16 +1298,34 @@ intel_hdmi_mode_valid(struct drm_connector *connector,
>   
>   static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)  {
> -	struct drm_device *dev = crtc_state->base.crtc->dev;
> +	struct drm_i915_private *dev_priv =
> +		to_i915(crtc_state->base.crtc->dev);
> +	struct drm_atomic_state *state = crtc_state->base.state;
> +	struct drm_connector_state *connector_state;
> +	struct drm_connector *connector;
> +	int i;
>   
> -	if (HAS_GMCH_DISPLAY(to_i915(dev)))
> +	if (HAS_GMCH_DISPLAY(dev_priv))
>   		return false;
>   
>   	/*
>   	 * HDMI 12bpc affects the clocks, so it's only possible
>   	 * when not cloning with other encoder types.
>   	 */
> -	return crtc_state->output_types == 1 << INTEL_OUTPUT_HDMI;
> +	if (crtc_state->output_types != 1 << INTEL_OUTPUT_HDMI)
> +		return false;
> +
This function is called from only one place ( for now), and that already 
has a pipe_config->has_hdmi_sink check.
Does it makes sense to have only one of the checks ? I can understand 
that this might be to comple
> +	for_each_connector_in_state(state, connector, connector_state, i) {
> +		const struct drm_display_info *info = &connector->display_info;
> +
> +		if (connector_state->crtc != crtc_state->base.crtc)
> +			continue;
> +
> +		if ((info->edid_hdmi_dc_modes & DRM_EDID_HDMI_DC_36) == 0)
> +			return false;
Do we want to have a info->bpc check too, or they are more or less same ?
- Shashank
> +	}
> +
> +	return true;
>   }
>   
>   bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> --
> 2.10.2
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel