[Intel-gfx] [PATCH v2 6/6] drm/i915: Set DP min_bpp to 8*3 for non-RGB output formats

[Ville Syrjälä]

On Thu, Apr 11, 2019 at 08:33:08PM +0000, Sripada, Radhakrishna wrote:
> On Thu, 2019-04-11 at 21:27 +0300, Ville Syrjälä wrote:
> > On Tue, Apr 09, 2019 at 02:04:01PM -0700, Dhinakaran Pandiyan wrote:
> > > On Tue, 2019-04-09 at 23:38 +0300, Ville Syrjälä wrote:
> > > > On Tue, Apr 09, 2019 at 01:28:18PM -0700, Dhinakaran Pandiyan
> > > > wrote:
> > > > > On Tue, 2019-03-26 at 16:25 +0200, Ville Syrjala wrote:
> > > > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > > 
> > > > > > 6bpc is only legal for RGB and RAW pixel encodings. For the
> > > > > > rest
> > > > > > the minimum is 8bpc. Set our lower limit accordingly.
> > > > > 
> > > > > Patch doesn't apply anymore, got a conflict in intel_drv.h. 
> > > > > 
> > > > > 
> > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_dp.c     | 10 +++++++++-
> > > > > >  drivers/gpu/drm/i915/intel_dp_mst.c |  2 +-
> > > > > >  drivers/gpu/drm/i915/intel_drv.h    |  1 +
> > > > > >  3 files changed, 11 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > index 2aee526ed632..149fdfbcb343 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > > > @@ -2002,6 +2002,14 @@ static int
> > > > > > intel_dp_dsc_compute_config(struct
> > > > > > intel_dp
> > > > > > *intel_dp,
> > > > > >  	return 0;
> > > > > >  }
> > > > > >  
> > > > > > +int intel_dp_min_bpp(const struct intel_crtc_state
> > > > > > *crtc_state)
> > > > > > +{
> > > > > > +	if (crtc_state->output_format ==
> > > > > > INTEL_OUTPUT_FORMAT_RGB)
> > > > > > +		return 6 * 3;
> > > > > > +	else
> > > > > > +		return 8 * 3;
> > > > > 
> > > > > Code matches spec, however I think there is a possibility of
> > > > > min_bpp
> > > > > becoming
> > > > > greater than max_bpp. The max_bpc property allows user space to
> > > > > set a value
> > > > > of 6
> > > > > and limits.min_bpp can become 24 because of the code above. Add
> > > > > a check for
> > > > > that
> > > > > in compute_link_config()? Probably would mess up the
> > > > > compute_config() loop
> > > > > too.
> > > > 
> > > > The code looks correct. Ie. should just end up with -EINVAL.
> > > 
> > > Yup, it does now as I read it carefully again :)
> > > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> > 
> > Ta. Pushed.
> Late on jumping the train but dont we have to limit the range exposed
> while attaching the "max bpc" as well in this case?

Late answering too. No we can't limit the range because we don't know
ahead of time whether RGB or YCbCr is going to be used. Well, we could
reject 6bpc entirely but that seems a bit silly too. The atomic check
will simply fail if you try a combo that doesn't work.

-- 
Ville Syrjälä
Intel