[Intel-gfx] [PATCH 15/15] drm/i915: implement fdi auto-dithering

[Daniel Vetter]

On Mon, Apr 29, 2013 at 05:59:53PM +0300, Imre Deak wrote:
> On Mon, 2013-04-29 at 16:43 +0200, Daniel Vetter wrote:
> > On Mon, Apr 29, 2013 at 05:02:20PM +0300, Imre Deak wrote:
> > > On Fri, 2013-04-19 at 11:24 +0200, Daniel Vetter wrote:

[snip]

> > > > @@ -7714,10 +7733,23 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> > > >  		}
> > > >  	}
> > > >  
> > > > -	if (!(intel_crtc_compute_config(crtc, pipe_config))) {
> > > > +	ret = intel_crtc_compute_config(crtc, pipe_config);
> > > > +	if (ret < 0) {
> > > >  		DRM_DEBUG_KMS("CRTC fixup failed\n");
> > > >  		goto fail;
> > > >  	}
> > > > +
> > > > +	if (ret == RETRY) {
> > > > +		if (WARN(!retry, "loop in pipe configuration computation\n")) {
> > > > +			ret = -EINVAL;
> > > > +			goto fail;
> > > 
> > > Isn't it possible that intel_dp_compute_config increases pipe_bpp when
> > > it forces pipe_bpp to what the firmware has set? In that case could hit
> > > this WARN.
> > 
> > Yeah, but that would mean that the firmware asks us for a configuration
> > for which we simply do not have enough bandwidth. So I don't think that
> > we'll actually hit this in reality.
> > 
> > If we do I guess we need to add a check into intel_dp_compute_config to
> > see whether we're in the a) retry loop and b) try to increase bpp and then
> > just fail the compute_config stage. But as long as the entire eDP mess is
> > a bit unclear I'd like to avoid such complexity until we have a proven
> > need.
> > 
> > Want me to add something like the above to the commit message?
> 
> Ok, after the IRC discussion things are clearer now. Yea, would be good
> to have have it in the commit message that eDP is a non-issue. With
> that:
> 
> Reviewed-by: Imre Deak <imre.deak at intel.com>

Ok, slurped in the entire series, with this patch's commit message amended
to reflect our discussion.

Thanks for the review, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch