Re: [PATCH] drm/i915: revert eDP bpp clamping code changes

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 04, 2013 at 03:44:01PM -0300, Paulo Zanoni wrote:
> Hi
> 
> 2013/4/4 Daniel Vetter <daniel.vetter@ffwll.ch>
> >
> > The behaviour around handling the eDP bpp value from vbt has been
> > slightly changed in
> >
> > commit 3600836585e3fdef0a1410d63fe5ce4015007aac
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Wed Mar 27 00:44:59 2013 +0100
> >
> >     drm/i915: convert DP autodither code to new infrastructure
> >
> > The old behaviour was that we used the plane's bpp (usually 24bpp) for
> > computing the dp link bw, but set up the pipe with the bpp value from
> > vbt if available. The takes the vbt bpp override into account even for
> s/The/This/ ?
> 
> > the dp link bw configuration.
> >
> > On Paulo's hsw machine this resulted in a slower link clock and a
> > black screen - but the mode actually /should/ fit even with the lower
> > clock. Until we've cleared up simply stay bug-for-bug compatible with
> > the old code.
> >
> > While at it, also restore a debug message lost in:
> >
> > commit 4e53c2e010e531b4a014692199e978482d471c7e
> > Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Date:   Wed Mar 27 00:44:58 2013 +0100
> >
> >     drm/i915: precompute pipe bpp before touching the hw
> >
> > Cc: Paulo Zanoni <przanoni@gmail.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c |   18 ++++++++++++++----
> >  1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index c5cfec3..658d071 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -741,9 +741,6 @@ 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 = min_t(int, 8*3, pipe_config->pipe_bpp);
> > -       if (is_edp(intel_dp) && dev_priv->edp.bpp)
> > -               bpp = min_t(int, bpp, dev_priv->edp.bpp);
> > -
> >         for (; bpp >= 6*3; bpp -= 2*3) {
> >                 mode_rate = intel_dp_link_required(target_clock, bpp);
> >
> > @@ -781,7 +778,6 @@ found:
> >         intel_dp->link_bw = bws[clock];
> >         intel_dp->lane_count = lane_count;
> >         adjusted_mode->clock = drm_dp_bw_code_to_link_rate(intel_dp->link_bw);
> > -       pipe_config->pipe_bpp = bpp;
> >         pipe_config->pixel_target_clock = target_clock;
> >
> >         DRM_DEBUG_KMS("DP link bw %02x lane count %d clock %d bpp %d\n",
> > @@ -796,6 +792,20 @@ found:
> >
> >         intel_dp_set_clock(encoder, pipe_config, intel_dp->link_bw);
> >
> > +       /*
> > +        * XXX: We have a strange regression where using the vbt edp bpp value
> > +        * for the link bw computation results in black screens, the panel only
> > +        * works when we do the computation at the usual 24bpp (but still
> > +        * requires us to use 18bpp. Until that's fully debugged, stay
> 
> We're missing a ')' character.
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Tested-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Fixed up and merged to dinq, thanks for digging into this an reviewing the
patch.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch