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
