Re: [PATCH 4/7] drm/i915: use pipe_config for lvds dithering

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Apr 25, 2013 at 02:24:50PM +0200, Daniel Vetter wrote:
> On Thu, Apr 25, 2013 at 02:57:16PM +0300, Ville Syrjälä wrote:
> > On Fri, Apr 19, 2013 at 11:14:34AM +0200, Daniel Vetter wrote:
> > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > > index 563f505..58a98ff 100644
> > > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > > @@ -136,7 +136,7 @@ static void intel_pre_pll_enable_lvds(struct intel_encoder *encoder)
> > >  	 * special lvds dither control bit on pch-split platforms, dithering is
> > >  	 * only controlled through the PIPECONF reg. */
> > >  	if (INTEL_INFO(dev)->gen == 4) {
> > > -		if (dev_priv->lvds_dither)
> > > +		if (intel_crtc->config.dither)
> > >  			temp |= LVDS_ENABLE_DITHER;
> > >  		else
> > >  			temp &= ~LVDS_ENABLE_DITHER;
> > > @@ -335,7 +335,13 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> > >  		DRM_DEBUG_KMS("forcing display bpp (was %d) to LVDS (%d)\n",
> > >  			      pipe_config->pipe_bpp, lvds_bpp);
> > >  		pipe_config->pipe_bpp = lvds_bpp;
> > > +
> > > +		/* Make sure pre-965 set dither correctly */
> > > +		if (INTEL_INFO(dev)->gen < 4)
> > > +			pfit_control |= PANEL_8TO6_DITHER_ENABLE;
> > 
> > I'm not quite sure about the gen4 and earlier stuff. Isn't the pipe
> > always 8bpc, and then we should enable dithering on the port/pfit
> > when lvds is 6bpc.
> > 
> > Right now I think we'll start with pipe_bpp=18 when the primary plane
> > surface is 16bpp, and then we wouldn't enable dithering here for 6bpc
> > lvds.
> 
> Yeah, the patch does slightly change behaviour as we no longer blindly
> follow the bios wrt dithering lvds. And imo trying to dither a 16bpp plane
> (even if the pipe is running internally at 8bpc) is a bit pointless, since
> there's simply no intermediate levels to dither down to 6bpc. Otoh just
> using the dither flag unconditionally gives us a notch more unified code.
> So I've opted for that.

I was just wondering what happens when we have 16bpp surface so
pipe_bpp is 18, and then we have 24bit lvds which means this hunk of code
will enable PANEL_8TO6_DITHER_ENABLE. Is that going to produce something
that still looks sensible?

-- 
Ville Syrjälä
Intel OTC