| Message ID | 1436260547-1879-2-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Op 07-07-15 om 11:15 schreef Daniel Vetter: > Since > > commit 8c7b5ccb729870e606321b3703e2c2e698c49a95 > Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > Date: Tue Apr 21 17:13:19 2015 +0300 > > drm/i915: Use atomic helpers for computing changed flags > > we compute the plane state for a modeset before actually committing > any changes, which means crtc->active won't be correct yet. Looking at > future work in the modeset conversion targetting 4.3 the only places > where crtc_state->active isn't accurate is when disabling other CRTCs > than the one the modeset is for (when stealing connectors). Which > isn't the case here. And that's also confirmed by an audit, we do > unconditionally update crtc_state->active for the current pipe. > > We also don't need to update any other plane check functions since we > only ever add the primary state to the modeset update right now. > > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 647b1404c441..ba9321998a41 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -13276,7 +13276,7 @@ intel_check_primary_plane(struct drm_plane *plane, > if (ret) > return ret; > > - if (intel_crtc->active) { > + if (crtc_state->base.active) { > struct intel_plane_state *old_state = > to_intel_plane_state(plane->state); > I think this was probably a feature, not a bug. Since full atomic planes won't be part of v4.2 both patches look ok to me. Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
On Wed, Jul 08, 2015 at 12:24:07PM +0200, Maarten Lankhorst wrote: > Op 07-07-15 om 11:15 schreef Daniel Vetter: > > Since > > > > commit 8c7b5ccb729870e606321b3703e2c2e698c49a95 > > Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > Date: Tue Apr 21 17:13:19 2015 +0300 > > > > drm/i915: Use atomic helpers for computing changed flags > > > > we compute the plane state for a modeset before actually committing > > any changes, which means crtc->active won't be correct yet. Looking at > > future work in the modeset conversion targetting 4.3 the only places > > where crtc_state->active isn't accurate is when disabling other CRTCs > > than the one the modeset is for (when stealing connectors). Which > > isn't the case here. And that's also confirmed by an audit, we do > > unconditionally update crtc_state->active for the current pipe. > > > > We also don't need to update any other plane check functions since we > > only ever add the primary state to the modeset update right now. > > > > Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Jani Nikula <jani.nikula@linux.intel.com> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 647b1404c441..ba9321998a41 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -13276,7 +13276,7 @@ intel_check_primary_plane(struct drm_plane *plane, > > if (ret) > > return ret; > > > > - if (intel_crtc->active) { > > + if (crtc_state->base.active) { > > struct intel_plane_state *old_state = > > to_intel_plane_state(plane->state); > > > I think this was probably a feature, not a bug. Since full atomic planes won't be part of v4.2 > both patches look ok to me. I wasn't sure - in dinq we didn't convert his as part of the patches which switched to check state->active in check functions, but only later on was removed in some seemingly unrelated refactor. Not sure why this didn't blow up more ... > Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Both applied, thanks for review. -Daniel
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 647b1404c441..ba9321998a41 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -13276,7 +13276,7 @@ intel_check_primary_plane(struct drm_plane *plane, if (ret) return ret; - if (intel_crtc->active) { + if (crtc_state->base.active) { struct intel_plane_state *old_state = to_intel_plane_state(plane->state);
Since commit 8c7b5ccb729870e606321b3703e2c2e698c49a95 Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Date: Tue Apr 21 17:13:19 2015 +0300 drm/i915: Use atomic helpers for computing changed flags we compute the plane state for a modeset before actually committing any changes, which means crtc->active won't be correct yet. Looking at future work in the modeset conversion targetting 4.3 the only places where crtc_state->active isn't accurate is when disabling other CRTCs than the one the modeset is for (when stealing connectors). Which isn't the case here. And that's also confirmed by an audit, we do unconditionally update crtc_state->active for the current pipe. We also don't need to update any other plane check functions since we only ever add the primary state to the modeset update right now. Cc: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)