Re: [Intel-gfx] [PATCH 2/7] drm/i915/fbc: replace a loop with drm_atomic_get_existing_crtc_state()
On Fri, Nov 11, 2016 at 05:46:16PM +, Chris Wilson wrote: > On Fri, Nov 11, 2016 at 02:57:36PM -0200, Paulo Zanoni wrote: > > Much simpler. Thanks to Ville for pointing this. > > > > Cc: Ville Syrjälä > > Reported-by: Ville Syrjälä > > Signed-off-by: Paulo Zanoni > > --- > > drivers/gpu/drm/i915/intel_fbc.c | 17 +++-- > > 1 file changed, 7 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c > > b/drivers/gpu/drm/i915/intel_fbc.c > > index ded77bd..b53b884 100644 > > --- a/drivers/gpu/drm/i915/intel_fbc.c > > +++ b/drivers/gpu/drm/i915/intel_fbc.c > > @@ -1071,7 +1071,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private > > *dev_priv, > > struct drm_plane *plane; > > struct drm_plane_state *plane_state; > > bool fbc_crtc_present = false; > > - int i, j; > > + int i; > > > > mutex_lock(&fbc->lock); > > > > @@ -1092,6 +1092,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private > > *dev_priv, > > for_each_plane_in_state(state, plane, plane_state, i) { > > struct intel_plane_state *intel_plane_state = > > to_intel_plane_state(plane_state); > > + struct intel_crtc_state *intel_crtc_state; > > > > if (!intel_plane_state->base.visible) > > continue; > > @@ -1099,16 +1100,12 @@ void intel_fbc_choose_crtc(struct drm_i915_private > > *dev_priv, > > if (!intel_fbc_can_choose(to_intel_crtc(plane_state->crtc))) > > continue; > > > > - for_each_crtc_in_state(state, crtc, crtc_state, j) { > > - struct intel_crtc_state *intel_crtc_state = > > - to_intel_crtc_state(crtc_state); > > - > > - if (plane_state->crtc != crtc) > > - continue; > > + intel_crtc_state = to_intel_crtc_state( > > + drm_atomic_get_existing_crtc_state(state, > > + plane_state->crtc)); > > My knowledge of atomic api is not good enough to say whether this can > return NULL at this point. If the plane is part of the state, then plane_state->crtc will be part of the state as well. Reviewed-by: Ville Syrjälä Just to reaffirm I wanted to point out what drm_atomic_set_crtc_for_plane() does, but looks like that one was written with the assumption that it could fail, which is not correct since drm_atomic_get_plane_state() will always add the current crtc to the state, and drm_atomic_set_crtc_for_plane() will always add the new crtc to the state. drm_atomic_set_crtc_for_connector() is written in the way I was expecting, so I think I'll fire off a patch for drm_atomic_set_crtc_for_plane() to follow the same pattern. -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/7] drm/i915/fbc: replace a loop with drm_atomic_get_existing_crtc_state()
On Fri, Nov 11, 2016 at 02:57:36PM -0200, Paulo Zanoni wrote: > Much simpler. Thanks to Ville for pointing this. > > Cc: Ville Syrjälä > Reported-by: Ville Syrjälä > Signed-off-by: Paulo Zanoni > --- > drivers/gpu/drm/i915/intel_fbc.c | 17 +++-- > 1 file changed, 7 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_fbc.c > b/drivers/gpu/drm/i915/intel_fbc.c > index ded77bd..b53b884 100644 > --- a/drivers/gpu/drm/i915/intel_fbc.c > +++ b/drivers/gpu/drm/i915/intel_fbc.c > @@ -1071,7 +1071,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private > *dev_priv, > struct drm_plane *plane; > struct drm_plane_state *plane_state; > bool fbc_crtc_present = false; > - int i, j; > + int i; > > mutex_lock(&fbc->lock); > > @@ -1092,6 +1092,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private > *dev_priv, > for_each_plane_in_state(state, plane, plane_state, i) { > struct intel_plane_state *intel_plane_state = > to_intel_plane_state(plane_state); > + struct intel_crtc_state *intel_crtc_state; > > if (!intel_plane_state->base.visible) > continue; > @@ -1099,16 +1100,12 @@ void intel_fbc_choose_crtc(struct drm_i915_private > *dev_priv, > if (!intel_fbc_can_choose(to_intel_crtc(plane_state->crtc))) > continue; > > - for_each_crtc_in_state(state, crtc, crtc_state, j) { > - struct intel_crtc_state *intel_crtc_state = > - to_intel_crtc_state(crtc_state); > - > - if (plane_state->crtc != crtc) > - continue; > + intel_crtc_state = to_intel_crtc_state( > + drm_atomic_get_existing_crtc_state(state, > +plane_state->crtc)); My knowledge of atomic api is not good enough to say whether this can return NULL at this point. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/7] drm/i915/fbc: replace a loop with drm_atomic_get_existing_crtc_state()
Much simpler. Thanks to Ville for pointing this. Cc: Ville Syrjälä Reported-by: Ville Syrjälä Signed-off-by: Paulo Zanoni --- drivers/gpu/drm/i915/intel_fbc.c | 17 +++-- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index ded77bd..b53b884 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -1071,7 +1071,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, struct drm_plane *plane; struct drm_plane_state *plane_state; bool fbc_crtc_present = false; - int i, j; + int i; mutex_lock(&fbc->lock); @@ -1092,6 +1092,7 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, for_each_plane_in_state(state, plane, plane_state, i) { struct intel_plane_state *intel_plane_state = to_intel_plane_state(plane_state); + struct intel_crtc_state *intel_crtc_state; if (!intel_plane_state->base.visible) continue; @@ -1099,16 +1100,12 @@ void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv, if (!intel_fbc_can_choose(to_intel_crtc(plane_state->crtc))) continue; - for_each_crtc_in_state(state, crtc, crtc_state, j) { - struct intel_crtc_state *intel_crtc_state = - to_intel_crtc_state(crtc_state); - - if (plane_state->crtc != crtc) - continue; + intel_crtc_state = to_intel_crtc_state( + drm_atomic_get_existing_crtc_state(state, + plane_state->crtc)); - intel_crtc_state->enable_fbc = true; - goto out; - } + intel_crtc_state->enable_fbc = true; + break; } out: -- 2.7.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx