[Intel-gfx] [PATCH v8 10/12] drm/i915: Update bits per component for display info

[Kahola, Mika]

Thanks for the review. I'll fix those indentations.

> -----Original Message-----
> From: Jim Bride [mailto:jim.bride at linux.intel.com]
> Sent: Thursday, September 8, 2016 12:27 AM
> To: Kahola, Mika <mika.kahola at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org;
> ville.syrjala at linux.intel.com; daniel.vetter at ffwll.ch
> Subject: Re: [PATCH v8 10/12] drm/i915: Update bits per component for
> display info
> 
> On Wed, Aug 17, 2016 at 01:49:47PM +0300, Mika Kahola wrote:
> > DisplayPort branch device may define max supported bits per component.
> > Update display info based on this value if bpc is defined.
> >
> > v2: cleanup to match the drm_dp_helper.c patches introduced
> >     earlier in this series
> > v3: Fill bpc for connector's display info in separate
> >     drm_dp_helper function (Daniel)
> > v4: remove updating bpc for display info as it may be overridden
> >     when parsing EDID. Instead, check bpc for DP branch device
> >     during compute_config
> >
> > Signed-off-by: Mika Kahola <mika.kahola at intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 17 ++++++++++++++++-
> >  1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c
> > b/drivers/gpu/drm/i915/intel_dp.c index 25f459e..17110d1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1524,6 +1524,20 @@ void intel_dp_compute_rate(struct intel_dp
> *intel_dp, int port_clock,
> >  	}
> >  }
> >
> > +int intel_dp_compute_bpp(struct intel_dp *intel_dp,
> > +			 struct intel_crtc_state *pipe_config)
> 
> Indentation seems off.
> 
> > +{
> > +	int bpp, bpc;
> > +
> > +	bpp = pipe_config->pipe_bpp;
> > +	bpc = drm_dp_downstream_max_bpc(intel_dp->dpcd,
> > +intel_dp->downstream_ports);
> > +
> > +	if (bpc > 0)
> 
> Do we need to ensure that bpp is sane before this calculation as well?
drm_dp_downstream_max_bpc() routine returns 0 if we can't find bpc from DPCD. Therefore bpc sanity is checked so to ensure that bpp has some meaningful value other than 0.

> 
> > +		bpp = min(bpp, 3*bpc);
> > +
> > +	return bpp;
> > +}
> > +
> >  bool
> >  intel_dp_compute_config(struct intel_encoder *encoder,
> >  			struct intel_crtc_state *pipe_config)
> 
> Indentation again.
> 
> Jim
> 
> > @@ -1589,7 +1603,8 @@ intel_dp_compute_config(struct intel_encoder
> > *encoder,
> >
> >  	/* Walk through all bpp values. Luckily they're all nicely spaced with 2
> >  	 * bpc in between. */
> > -	bpp = pipe_config->pipe_bpp;
> > +	bpp = intel_dp_compute_bpp(intel_dp, pipe_config);
> > +
> >  	if (is_edp(intel_dp)) {
> >
> >  		/* Get bpp from vbt only for panels that dont have bpp in
> edid */
> > --
> > 1.9.1