From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 4/7] drm/i915: use pipe_config for lvds dithering Date: Thu, 25 Apr 2013 15:42:47 +0300 Message-ID: <20130425124247.GD4469@intel.com> References: <1366362877-15446-1-git-send-email-daniel.vetter@ffwll.ch> <1366362877-15446-5-git-send-email-daniel.vetter@ffwll.ch> <20130425115715.GZ4469@intel.com> <20130425122450.GC6169@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga14.intel.com (mga14.intel.com [143.182.124.37]) by gabe.freedesktop.org (Postfix) with ESMTP id 3CF9FE5C14 for ; Thu, 25 Apr 2013 05:42:51 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130425122450.GC6169@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org 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=E4l=E4 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 inte= l_encoder *encoder) > > > * special lvds dither control bit on pch-split platforms, ditherin= g is > > > * only controlled through the PIPECONF reg. */ > > > if (INTEL_INFO(dev)->gen =3D=3D 4) { > > > - if (dev_priv->lvds_dither) > > > + if (intel_crtc->config.dither) > > > temp |=3D LVDS_ENABLE_DITHER; > > > else > > > temp &=3D ~LVDS_ENABLE_DITHER; > > > @@ -335,7 +335,13 @@ static bool intel_lvds_compute_config(struct int= el_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 =3D lvds_bpp; > > > + > > > + /* Make sure pre-965 set dither correctly */ > > > + if (INTEL_INFO(dev)->gen < 4) > > > + pfit_control |=3D 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=3D18 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=E4l=E4 Intel OTC