From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: fix pfit regression for non-autoscaled resolutions Date: Fri, 12 Jul 2013 18:56:25 +0200 Message-ID: <20130712165625.GH6143@phenom.ffwll.local> References: <1373609250-17663-1-git-send-email-daniel.vetter@ffwll.ch> <20130712080416.016b0677@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f170.google.com (mail-ea0-f170.google.com [209.85.215.170]) by gabe.freedesktop.org (Postfix) with ESMTP id AFF9CE6816 for ; Fri, 12 Jul 2013 09:56:30 -0700 (PDT) Received: by mail-ea0-f170.google.com with SMTP id h10so6674160eaj.1 for ; Fri, 12 Jul 2013 09:56:29 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130712080416.016b0677@jbarnes-desktop> 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: Jesse Barnes Cc: Daniel Vetter , Intel Graphics Development , Hans de Bruin , Mika Kuoppala List-Id: intel-gfx@lists.freedesktop.org On Fri, Jul 12, 2013 at 08:04:16AM -0700, Jesse Barnes wrote: > On Fri, 12 Jul 2013 08:07:30 +0200 > Daniel Vetter wrote: > > > I.e. for letter/pillarboxing. For those cases we need to adjust the > > mode a bit, but Jesse gmch pfit refactoring in > > > > commit 2dd24552cab40ea829ba3fda890eeafd2c4816d8 > > Author: Jesse Barnes > > Date: Thu Apr 25 12:55:01 2013 -0700 > > > > drm/i915: factor out GMCH panel fitting code and use for eDP v3 > > > > broke that by reordering the computation of the gmch pfit state with > > the block of code that prepared the adjusted mode for it and told the > > modeset core not to overwrite the adjusted mode with default settings. > > > > We might want to switch around the core code to just fill in defaults, > > but this code predates the pipe_config modeset rework. And in the old > > crtc helpers we did not have a suitable spot to do this. > > > > Cc: Mika Kuoppala > > Cc: Jesse Barnes > > Cc: Hans de Bruin > > Reported-and-tested-by: Hans de Bruin > > Signed-off-by: Daniel Vetter > > --- > > drivers/gpu/drm/i915/intel_lvds.c | 5 +---- > > drivers/gpu/drm/i915/intel_panel.c | 3 +++ > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > > index b0e1088..0536c9b 100644 > > --- a/drivers/gpu/drm/i915/intel_lvds.c > > +++ b/drivers/gpu/drm/i915/intel_lvds.c > > @@ -297,14 +297,11 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, > > > > intel_pch_panel_fitting(intel_crtc, pipe_config, > > intel_connector->panel.fitting_mode); > > - return true; > > } else { > > intel_gmch_panel_fitting(intel_crtc, pipe_config, > > intel_connector->panel.fitting_mode); > > - } > > > > - drm_mode_set_crtcinfo(adjusted_mode, 0); > > - pipe_config->timings_set = true; > > + } > > > > /* > > * XXX: It would be nice to support lower refresh rates on the > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > > index 80bea1d..45010bb 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -194,6 +194,9 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc, > > adjusted_mode->vdisplay == mode->vdisplay) > > goto out; > > > > + drm_mode_set_crtcinfo(adjusted_mode, 0); > > + pipe_config->timings_set = true; > > + > > switch (fitting_mode) { > > case DRM_MODE_SCALE_CENTER: > > /* > > This code is a bit confusing, but this looks better than what was there > before (clobbering the adjusted_mode after calling gmch_panel_fitting > definitely seems wrong). Only nit is that the timings_set flag isn't > very descriptive, but that's not the fault of this patch. Yeah, that's just an artifact from the old crtc helper code, with the new compute_config callbacks we could precompute sane crtc timings. I'll fix this up for 3.12 > Reviewed-by: Jesse Barnes Picked up for -fixes, thanks for the review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch