Re: [PATCH] Implement Limited Video Range

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 30, 2015 at 08:08:48AM +0100, Peter Frühberger wrote:
> (Resent cause of moderation)
> 
> This implements a highly needed feature in a minimal non instructive way.
> Consider a Limited Range display (like most TVs) where you want to watch a
> decoded video. The TV is set to limited range and the intel driver also
> uses full scaling Limited 16:235 mode, e.g. if you display the gray value
> 16 - the driver will postprocess it to something like ~ 22.
> 
> The following happens currently in linux intel video world:
> Video is decoded with e.g. vaapi, the decoded surface is either used via
> EGL directly in Limited Range. Limited Range processing takes place and at
> the end while rendering the intel driver will scale down the limited range
> data again as it cannot distunguish of the data it gets when e.g. rendering
> with OpenGL like we succesfully do in kodi.
> 
> Another use case is vaapi when using the old vaCopySurfaceGLX
> (vaPutSurface) which would automatically use BT601 / BT709 matrix to
> upscale the content without any dithering to Full Range RGB. Later on this
> is scaled down again with the Limited 16:235 setting and output. Quality
> degrades two times.
> 
> Users and applications widely used want to make sure that colors appear on
> screen like they were processed after the last processing step. In video
> case two use cases make sense:
> 
> a) scaling limited range to full range with dithering applied when you need
> to output fullrange
> b) already having limited range and outputting that without any further
> touching by the driver
> 
> Use case a) is already possible. Usecase b) will be possible with the
> attached patch. It is a possibility to get Limited Range on screen in
> perfect quality in 2k15.
> 
> For the future a userspace api to provide info frames and so on in a
> generic way should be considered but until we have this I bet we have 2
> years to go. This solution also works on X11 (without wayland) and is
> useful for existing applications.
> 
> Thanks much for consideration.
> 
> ---
> From deaa9d730c08aefefe3671d073d93d970bb39a31 Mon Sep 17 00:00:00 2001
> From: fritsch <peter.fruehberger@xxxxxxxxx>
> Date: Sun, 29 Nov 2015 16:38:14 +0100
> Subject: [PATCH] Intel: Implement Video Color Range
> 
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c |  4 ++--
>  drivers/gpu/drm/i915/intel_drv.h     |  8 ++++++++
>  drivers/gpu/drm/i915/intel_hdmi.c    | 17 +++++++++++++++--
>  drivers/gpu/drm/i915/intel_modes.c   |  1 +
>  5 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 95bb27d..e453461 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3449,6 +3449,7 @@ int intel_freq_opcode(struct drm_i915_private
> *dev_priv, int val);
>  #define INTEL_BROADCAST_RGB_AUTO 0
>  #define INTEL_BROADCAST_RGB_FULL 1
>  #define INTEL_BROADCAST_RGB_LIMITED 2
> +#define INTEL_BROADCAST_RGB_VIDEO 3
> 
>  static inline uint32_t i915_vgacntrl_reg(struct drm_device *dev)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 71860f8..ea40d81 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8605,7 +8605,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
>    * consideration.
>    */
> 
> - if (intel_crtc->config->limited_color_range)
> + if (intel_crtc->config->limited_color_range &&
> !intel_crtc->config->video_color_range)
>   coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */
> 
>   /*
> @@ -8629,7 +8629,7 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
>   if (INTEL_INFO(dev)->gen > 6) {
>   uint16_t postoff = 0;
> 
> - if (intel_crtc->config->limited_color_range)
> + if (intel_crtc->config->limited_color_range &&
> !intel_crtc->config->video_color_range)
>   postoff = (16 * (1 << 12) / 255) & 0x1fff;
> 
>   I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 0598932..6940243 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -372,6 +372,13 @@ struct intel_crtc_state {
>    */
>   bool limited_color_range;
> 
> + /*
> +  *
> +  * Use reduced/limited/broadcast rgb range without compressing.
> +  *
> +  */
> + bool video_color_range;

I think the names and semantics are confusion, resulting in hard-to-read
if conditions. What about:

	bool limit_color_range; /* should we limit from full->16:235 in
	the encoder/csc? */

	bool limited_color_range_output; /* set the infoframe or not? */

Names aren't great yet I think, coffee's not yet working ;-)

Thanks, Daniel

> +
>   /* DP has a bunch of special case unfortunately, so mark the pipe
>    * accordingly. */
>   bool has_dp_encoder;
> @@ -682,6 +689,7 @@ struct intel_hdmi {
>   int ddc_bus;
>   bool limited_color_range;
>   bool color_range_auto;
> + bool color_range_video;
>   bool has_hdmi_sink;
>   bool has_audio;
>   enum hdmi_force_audio force_audio;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 9eafa19..dc78760 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -464,7 +464,8 @@ static void intel_hdmi_set_avi_infoframe(struct
> drm_encoder *encoder,
>   }
> 
>   if (intel_hdmi->rgb_quant_range_selectable) {
> - if (intel_crtc->config->limited_color_range)
> + if (intel_crtc->config->limited_color_range ||
> +     intel_crtc->config->video_color_range)
>   frame.avi.quantization_range =
>   HDMI_QUANTIZATION_RANGE_LIMITED;
>   else
> @@ -1266,6 +1267,8 @@ bool intel_hdmi_compute_config(struct intel_encoder
> *encoder,
>   pipe_config->limited_color_range =
>   intel_hdmi->limited_color_range;
>   }
> + if (intel_hdmi->color_range_video)
> +               pipe_config->video_color_range = true;
> 
>   if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) {
>   pipe_config->pixel_multiplier = 2;
> @@ -1485,25 +1488,35 @@ intel_hdmi_set_property(struct drm_connector
> *connector,
>   if (property == dev_priv->broadcast_rgb_property) {
>   bool old_auto = intel_hdmi->color_range_auto;
>   bool old_range = intel_hdmi->limited_color_range;
> + bool old_range_video = intel_hdmi->color_range_video;
> 
>   switch (val) {
>   case INTEL_BROADCAST_RGB_AUTO:
>   intel_hdmi->color_range_auto = true;
> + intel_hdmi->color_range_video = false;
>   break;
>   case INTEL_BROADCAST_RGB_FULL:
>   intel_hdmi->color_range_auto = false;
>   intel_hdmi->limited_color_range = false;
> + intel_hdmi->color_range_video = false;
>   break;
>   case INTEL_BROADCAST_RGB_LIMITED:
>   intel_hdmi->color_range_auto = false;
>   intel_hdmi->limited_color_range = true;
> + intel_hdmi->color_range_video = false;
> + break;
> + case INTEL_BROADCAST_RGB_VIDEO:
> +                        intel_hdmi->color_range_auto = false;
> +                        intel_hdmi->limited_color_range = true;
> +                        intel_hdmi->color_range_video = true;
>   break;
>   default:
>   return -EINVAL;
>   }
> 
>   if (old_auto == intel_hdmi->color_range_auto &&
> -     old_range == intel_hdmi->limited_color_range)
> +     old_range == intel_hdmi->limited_color_range &&
> +     old_range_video == intel_hdmi->color_range_video)
>   return 0;
> 
>   goto done;
> diff --git a/drivers/gpu/drm/i915/intel_modes.c
> b/drivers/gpu/drm/i915/intel_modes.c
> index 38a4c8c..c49681a 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -103,6 +103,7 @@ static const struct drm_prop_enum_list
> broadcast_rgb_names[] = {
>   { INTEL_BROADCAST_RGB_AUTO, "Automatic" },
>   { INTEL_BROADCAST_RGB_FULL, "Full" },
>   { INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
> + { INTEL_BROADCAST_RGB_VIDEO, "Video 16:235 pass-through" },
>  };
> 
>  void
> -- 
> 2.5.0

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


-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux