Re: [PATCH 07/11] drm: set output colorspace in AVI infoframe

[Jose Abreu <Jose.Abreu@synopsys.com>]

Hi Shashank,


On 07-04-2017 17:39, Shashank Sharma wrote:
> HDMI source must set output colorspace information in AVI
> infoframes, so that the sink can decode upcoming frames
> accordingly.
>
> As this patch series is adding support for HDMI output modes
> other than RGB, this patch adds a function in DRM layer, to
> add the output colorspace information in the AVI infoframes.
>
> Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> Cc: Jose Abreu <Jose.Abreu@synopsys.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 40 ++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_edid.h     |  5 +++++
>  2 files changed, 45 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 828b781..a02d35b 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4734,6 +4734,46 @@ 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_set_colorspace() - fill an HDMI AVI infoframe with
> + * colorspace data of the output type
> + *
> + * @frame: HDMI AVI infoframe
> + * @mode: DRM display mode
> + * @hdmi_output: HDMI output colorspace
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int
> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame,
> +					 const struct drm_display_mode *mode,
> +					 enum drm_hdmi_output_type hdmi_output)
> +{
> +	switch (hdmi_output) {
> +	case DRM_HDMI_OUTPUT_YCBCR444:
> +		frame->colorspace = HDMI_COLORSPACE_YUV444;
> +		break;
> +	case DRM_HDMI_OUTPUT_YCBCR422:
> +		frame->colorspace = HDMI_COLORSPACE_YUV422;
> +		break;
> +	case DRM_HDMI_OUTPUT_YCBCR420:
> +		frame->colorspace = HDMI_COLORSPACE_YUV420;
> +		frame->pixel_repeat = 0;

Why only 4:2:0 is set with pixel repetition off? Is this per spec?

> +		break;
> +	case DRM_HDMI_OUTPUT_DEFAULT_RGB:
> +		frame->colorspace = HDMI_COLORSPACE_RGB;
> +		break;
> +	case DRM_HDMI_OUTPUT_YCBCR_HQ:
> +	case DRM_HDMI_OUTPUT_YCBCR_LQ:
> +	case DRM_HDMI_OUTPUT_INVALID:

Maybe default to RGB here? I don't know if spec says anything
about sending empty colorspace field.

Best regards,
Jose Miguel Abreu

> +		DRM_ERROR("Invalid HDMI output type\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_set_colorspace);
> +
> +/**
>   * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
>   *                                        quantization range information
>   * @frame: HDMI AVI infoframe
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index a4d174e7..8980366 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -329,6 +329,7 @@ struct cea_sad {
>  struct drm_encoder;
>  struct drm_connector;
>  struct drm_display_mode;
> +enum drm_hdmi_output_type;
>  
>  void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
>  int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
> @@ -351,6 +352,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  					 const struct drm_display_mode *mode,
>  					 bool is_hdmi2);
>  int
> +drm_hdmi_avi_infoframe_set_colorspace(struct hdmi_avi_infoframe *frame,
> +					 const struct drm_display_mode *mode,
> +					 enum drm_hdmi_output_type hdmi_output);
> +int
>  drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>  					    const struct drm_display_mode *mode);
>  void

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx