From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 36/66] drm/i915: Only update shared dpll state when needed Date: Tue, 20 May 2014 13:17:11 +0200 Message-ID: <20140520111711.GO8790@phenom.ffwll.local> References: <1398376542-27825-1-git-send-email-daniel.vetter@ffwll.ch> <1398376542-27825-37-git-send-email-daniel.vetter@ffwll.ch> <20140520101854.GA12931@strange.amr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f43.google.com (mail-ee0-f43.google.com [74.125.83.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 8FD206E77B for ; Tue, 20 May 2014 04:17:16 -0700 (PDT) Received: by mail-ee0-f43.google.com with SMTP id d17so456360eek.16 for ; Tue, 20 May 2014 04:17:15 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140520101854.GA12931@strange.amr.corp.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Damien Lespiau Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Tue, May 20, 2014 at 11:18:54AM +0100, Damien Lespiau wrote: > On Thu, Apr 24, 2014 at 11:55:12PM +0200, Daniel Vetter wrote: > > Instead of every time it isn't active: We only need to do that when > > the pll is currently unused, i.e. when pll->refcount == 0. For > > paranoia add a warning for the ibx case where plls have a fixed > > mapping and hence should always be unused after the call to > > intel_put_shared_dpll. > > > > Signed-off-by: Daniel Vetter > > - Instead of duplicating the memcpy() I'd add a single path after the > 'found' label, > - You could ensure type safety in the copy: > > if (pll->refcount == 0) > pll->hw_state = crtc->config.dpll_hw_state I like this, so will quickly respin. -Daniel > > Oh well. > > Reviewed-by: Damien Lespiau > > -- > Damien > > > --- > > drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- > > 1 file changed, 9 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 21e20bc495ee..aef869dcc054 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -3416,6 +3416,11 @@ static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc) > > DRM_DEBUG_KMS("CRTC:%d using pre-allocated %s\n", > > crtc->base.base.id, pll->name); > > > > + WARN_ON(pll->refcount); > > + > > + memcpy(&pll->hw_state, &crtc->config.dpll_hw_state, > > + sizeof(pll->hw_state)); > > + > > goto found; > > } > > > > @@ -3442,6 +3447,10 @@ static struct intel_shared_dpll *intel_get_shared_dpll(struct intel_crtc *crtc) > > if (pll->refcount == 0) { > > DRM_DEBUG_KMS("CRTC:%d allocated %s\n", > > crtc->base.base.id, pll->name); > > + > > + memcpy(&pll->hw_state, &crtc->config.dpll_hw_state, > > + sizeof(pll->hw_state)); > > + > > goto found; > > } > > } > > @@ -3454,9 +3463,6 @@ found: > > pipe_name(crtc->pipe)); > > > > if (pll->active == 0) { > > - memcpy(&pll->hw_state, &crtc->config.dpll_hw_state, > > - sizeof(pll->hw_state)); > > - > > DRM_DEBUG_DRIVER("setting up %s\n", pll->name); > > WARN_ON(pll->on); > > assert_shared_dpll_disabled(dev_priv, pll); > > -- > > 1.8.1.4 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch