| Message ID | 1398376542-27825-37-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <daniel.vetter@ffwll.ch> - 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 Oh well. Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
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 <daniel.vetter@ffwll.ch> > > - 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.lespiau@intel.com> > > -- > 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
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);
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 <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/intel_display.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)