| Message ID | 20170712081344.25495-2-maarten.lankhorst@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 12, 2017 at 10:13:29AM +0200, Maarten Lankhorst wrote: > You can enable the CRTC and without adding the plane to the state and > it will succeed. This should be prevented in the crtc check instead of > the plane check, because the plane check may never run for atomic > enable, but the crtc check always will. > > This is based on a similar check in vmwgfx. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Sean Paul <seanpaul@chromium.org> > Cc: David Airlie <airlied@linux.ie> Somehow this missed to cc: Noralf ... I guess get_maintainers is not quite smart enough. Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> > --- > drivers/gpu/drm/drm_simple_kms_helper.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c > index 98250854af75..39c203ad59db 100644 > --- a/drivers/gpu/drm/drm_simple_kms_helper.c > +++ b/drivers/gpu/drm/drm_simple_kms_helper.c > @@ -37,6 +37,13 @@ static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = { > static int drm_simple_kms_crtc_check(struct drm_crtc *crtc, > struct drm_crtc_state *state) > { > + bool has_primary = state->plane_mask & > + BIT(drm_plane_index(crtc->primary)); > + > + /* We always want to have an active plane with an active CRTC */ > + if (has_primary != state->enable) > + return -EINVAL; > + > return drm_atomic_add_affected_planes(state->state, crtc); > } > > @@ -90,9 +97,6 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, > pipe = container_of(plane, struct drm_simple_display_pipe, plane); > crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, > &pipe->crtc); > - if (crtc_state->enable != !!plane_state->crtc) > - return -EINVAL; /* plane must match crtc enable state */ > - > if (!crtc_state->enable) > return 0; /* nothing to check when disabling or disabled */ > > -- > 2.11.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 98250854af75..39c203ad59db 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -37,6 +37,13 @@ static const struct drm_encoder_funcs drm_simple_kms_encoder_funcs = { static int drm_simple_kms_crtc_check(struct drm_crtc *crtc, struct drm_crtc_state *state) { + bool has_primary = state->plane_mask & + BIT(drm_plane_index(crtc->primary)); + + /* We always want to have an active plane with an active CRTC */ + if (has_primary != state->enable) + return -EINVAL; + return drm_atomic_add_affected_planes(state->state, crtc); } @@ -90,9 +97,6 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, pipe = container_of(plane, struct drm_simple_display_pipe, plane); crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, &pipe->crtc); - if (crtc_state->enable != !!plane_state->crtc) - return -EINVAL; /* plane must match crtc enable state */ - if (!crtc_state->enable) return 0; /* nothing to check when disabling or disabled */
You can enable the CRTC and without adding the plane to the state and it will succeed. This should be prevented in the crtc check instead of the plane check, because the plane check may never run for atomic enable, but the crtc check always will. This is based on a similar check in vmwgfx. Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Sean Paul <seanpaul@chromium.org> Cc: David Airlie <airlied@linux.ie> --- drivers/gpu/drm/drm_simple_kms_helper.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)