[Intel-gfx] [PATCH 4/7] drm/i915: use pipe_config for lvds dithering

[Daniel Vetter]

On Thu, Apr 25, 2013 at 2:42 PM, Ville Syrjälä
<ville.syrjala at linux.intel.com> wrote:
> 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?

Hm, indeed I've ignored this case a bit. I guess it would result in
something ok-ish, but I'm not sure. I'll do a similar check to what
I've added for g4x/vlv in another patch for 30bpp modes, and simply
only enabling dithering when we think it's needed and when we think
the "pipe" runs at 18bpp. I'll update the patch a bit.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch