Re: [Intel-gfx] [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Eric Anholt <eric@anholt.net>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
Date: Fri, 20 Jan 2017 22:37:16 +0200	[thread overview]
Message-ID: <20170120203716.GL31595@intel.com> (raw)
In-Reply-To: <20170120200034.GK31595@intel.com>

On Fri, Jan 20, 2017 at 10:00:34PM +0200, Ville Syrjälä wrote:
> On Sat, Jan 21, 2017 at 06:50:57AM +1100, Eric Anholt wrote:
> > ville.syrjala@linux.intel.com writes:
> > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Make the code selecting the RGB quantization range a little less magicy
> > > by wrapping it up in a small helper.
> > 
> > This series seems good.  I won't have the ability to test it on vc4
> > within a reasonable time frame, so you can add an Acked-by from me if
> > you'd like.  Just one comment that you can take or leave:
> > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
> > >  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
> > >  drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
> > >  include/drm/drm_edid.h            |  2 ++
> > >  5 files changed, 28 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index 4ff04aa84dd0..304c583b8000 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
> > >  }
> > >  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
> > >  
> > > +/**
> > > + * drm_default_rgb_quant_range - default RGB quantization range
> > > + * @mode: display mode
> > > + *
> > > + * Determine the default RGB quantization range for the mode,
> > > + * as specified in CEA-861.
> > > + *
> > > + * Return: The default RGB quantization range for the mode
> > > + */
> > > +enum hdmi_quantization_range
> > > +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> > > +{
> > > +	return drm_match_cea_mode(mode) > 1 ?
> > > +		HDMI_QUANTIZATION_RANGE_LIMITED :
> > > +		HDMI_QUANTIZATION_RANGE_FULL;
> > 
> > It might be nice to add a comment here like
> > 
> > /* All CEA modes other than VIC 1 use limited quantization range. */
> > 
> > When I first had to put this logic in vc4, I was surprised by it and
> > figured that it was an off-by-one bug in code that was trying to say "if
> > it's any CEA mode then it should be limited range"
> 
> Seems like a good idea. I could even try to dig up a specific spec
> quote perhaps? And if I can't find anything succinct in the spec we
> can always go with your proposal.

Meh. Didn't manage to dig up anything really nice from the spec, so
I'll just go with your idea.

> 
> > 
> > 
> > > +}
> > > +EXPORT_SYMBOL(drm_default_rgb_quant_range);
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel