From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 4/4] drm/i915: hsw backlight registers need transcoder instead of pipe Date: Thu, 25 Apr 2013 15:45:43 +0200 Message-ID: <20130425134543.GG6169@phenom.ffwll.local> References: <3469fd948f856b5b1223731e3b57d69e50fb00d5.1365768957.git.jani.nikula@intel.com> <1366895663.13545.10.camel@intelbox> <20130425133426.GE6169@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ea0-f175.google.com (mail-ea0-f175.google.com [209.85.215.175]) by gabe.freedesktop.org (Postfix) with ESMTP id 65FC5E5C73 for ; Thu, 25 Apr 2013 06:42:40 -0700 (PDT) Received: by mail-ea0-f175.google.com with SMTP id f15so1227484eak.34 for ; Thu, 25 Apr 2013 06:42:39 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130425133426.GE6169@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: Imre Deak Cc: Jani Nikula , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Apr 25, 2013 at 03:34:26PM +0200, Daniel Vetter wrote: > On Thu, Apr 25, 2013 at 04:14:23PM +0300, Imre Deak wrote: > > On Fri, 2013-04-12 at 15:18 +0300, Jani Nikula wrote: > > > Signed-off-by: Jani Nikula > > > --- > > > drivers/gpu/drm/i915/intel_panel.c | 4 +++- > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > > > index 5d3e9d7..0362f5c 100644 > > > --- a/drivers/gpu/drm/i915/intel_panel.c > > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > > @@ -344,6 +344,8 @@ void intel_panel_enable_backlight(struct drm_device *dev, > > > enum pipe pipe) > > > { > > > struct drm_i915_private *dev_priv = dev->dev_private; > > > + enum transcoder cpu_transcoder = > > > + intel_pipe_to_cpu_transcoder(dev_priv, pipe); > > > unsigned long flags; > > > > > > spin_lock_irqsave(&dev_priv->backlight.lock, flags); > > > @@ -374,7 +376,7 @@ void intel_panel_enable_backlight(struct drm_device *dev, > > > else > > > tmp &= ~BLM_PIPE_SELECT; > > > > > > - tmp |= BLM_PIPE(pipe); > > > + tmp |= BLM_PIPE(cpu_transcoder); > > > > Imo BLM_PIPE would be clearer checking for TRANSCODER_EDP explicitly, > > but the code works anyway, so: > > Reviewed-by: Imre Deak > > Atm all the difference between pipe and cpu_transcoder is about transcoder > edp. So this feels explicit enough for me. Imre pointed out that his concern is about the truncation, since for eDP this evaluates to (0xf << 29). Which truncates to the right value, but I agree it's a bit too much working-by-accident. Dropped the patch again. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch