Re: [PATCH 3/5] drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Pull the logic to populate the quantization range information
> in the AVI infoframe into a small helper. We'll be adding a bit
> more logic to it, and having it in a central place seems like a
> good idea since it's based on the CEA-861 spec.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c        | 26 ++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++--------
>  drivers/gpu/drm/vc4/vc4_hdmi.c    | 14 +++++---------
>  include/drm/drm_edid.h            |  4 ++++
>  4 files changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 304c583b8000..548c20250b95 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4291,6 +4291,32 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  }
>  EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
>  
> +/**
> + * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
> + *                                        quantization range information

I'm a fan of having a verb in the function name for functions that do
stuff. Would be nice to have "set" in there somewhere, although the name
is growing a bit unwieldy. (Btw, same for drm_default_rgb_quant_range()
in the previous patch, "get" would be nice.)

Up to you.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> + * @frame: HDMI AVI infoframe
> + * @rgb_quant_range: RGB quantization range (Q)
> + * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS)
> + */
> +void
> +drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> +				   enum hdmi_quantization_range rgb_quant_range,
> +				   bool rgb_quant_range_selectable)
> +{
> +	/*
> +	 * CEA-861:
> +	 * "A Source shall not send a non-zero Q value that does not correspond
> +	 *  to the default RGB Quantization Range for the transmitted Picture
> +	 *  unless the Sink indicates support for the Q bit in a Video
> +	 *  Capabilities Data Block."
> +	 */
> +	if (rgb_quant_range_selectable)
> +		frame->quantization_range = rgb_quant_range;
> +	else
> +		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
> +}
> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_quant_range);
> +
>  static enum hdmi_3d_structure
>  s3d_structure_from_display_mode(const struct drm_display_mode *mode)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 19bd13f53729..351f837b09a0 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -465,14 +465,11 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>  		return;
>  	}
>  
> -	if (intel_hdmi->rgb_quant_range_selectable) {
> -		if (crtc_state->limited_color_range)
> -			frame.avi.quantization_range =
> -				HDMI_QUANTIZATION_RANGE_LIMITED;
> -		else
> -			frame.avi.quantization_range =
> -				HDMI_QUANTIZATION_RANGE_FULL;
> -	}
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +					   crtc_state->limited_color_range ?
> +					   HDMI_QUANTIZATION_RANGE_LIMITED :
> +					   HDMI_QUANTIZATION_RANGE_FULL,
> +					   intel_hdmi->rgb_quant_range_selectable);
>  
>  	intel_write_infoframe(encoder, crtc_state, &frame);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index d79466a42690..a588156b5410 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -356,15 +356,11 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
>  		return;
>  	}
>  
> -	if (vc4_encoder->rgb_range_selectable) {
> -		if (vc4_encoder->limited_rgb_range) {
> -			frame.avi.quantization_range =
> -				HDMI_QUANTIZATION_RANGE_LIMITED;
> -		} else {
> -			frame.avi.quantization_range =
> -				HDMI_QUANTIZATION_RANGE_FULL;
> -		}
> -	}
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +					   vc4_encoder->limited_rgb_range ?
> +					   HDMI_QUANTIZATION_RANGE_LIMITED :
> +					   HDMI_QUANTIZATION_RANGE_FULL,
> +					   vc4_encoder->rgb_range_selectable);
>  
>  	vc4_hdmi_write_infoframe(encoder, &frame);
>  }
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 25cdf5f7a0d8..cfad4d89589f 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -345,6 +345,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  int
>  drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>  					    const struct drm_display_mode *mode);
> +void
> +drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> +				   enum hdmi_quantization_range rgb_quant_range,
> +				   bool rgb_quant_range_selectable);
>  
>  /**
>   * drm_eld_mnl - Get ELD monitor name length in bytes.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx