| Message ID | 1493214013-15580-9-git-send-email-imre.deak@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Apr 26, 2017 at 04:40:13PM +0300, Imre Deak wrote: > plane_state can't be NULL anywhere in this function, so the NULL check > at the end is redundant, remove it. > > Cc: dri-devel@lists.freedesktop.org > Signed-off-by: Imre Deak <imre.deak@intel.com> > --- > drivers/gpu/drm/drm_plane_helper.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c > index 9854a50..2c27f6f 100644 > --- a/drivers/gpu/drm/drm_plane_helper.c > +++ b/drivers/gpu/drm/drm_plane_helper.c > @@ -511,12 +511,10 @@ int drm_plane_helper_commit(struct drm_plane *plane, > if (plane_funcs->cleanup_fb) > plane_funcs->cleanup_fb(plane, plane_state); > out: > - if (plane_state) { > - if (plane->funcs->atomic_destroy_state) > - plane->funcs->atomic_destroy_state(plane, plane_state); > - else > - drm_atomic_helper_plane_destroy_state(plane, plane_state); > - } > + if (plane->funcs->atomic_destroy_state) > + plane->funcs->atomic_destroy_state(plane, plane_state); > + else > + drm_atomic_helper_plane_destroy_state(plane, plane_state); Hmm. If plane->state was NULL, then we could swap that with plane_state, and thus plane_state could become NULL. But that would actually oops in drm_atomic_plane_disabling() already, so yeah, no way this could work. My only concern is the buggy drm_atomic_helper_plane_reset() which can't fail gracefully if the kmalloc fails. But failure that would probably lead to explosions all over anyway. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > return ret; > } > -- > 2.5.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Apr 26, 2017 at 06:44:53PM +0300, Ville Syrjälä wrote: > On Wed, Apr 26, 2017 at 04:40:13PM +0300, Imre Deak wrote: > > plane_state can't be NULL anywhere in this function, so the NULL check > > at the end is redundant, remove it. > > > > Cc: dri-devel@lists.freedesktop.org > > Signed-off-by: Imre Deak <imre.deak@intel.com> > > --- > > drivers/gpu/drm/drm_plane_helper.c | 10 ++++------ > > 1 file changed, 4 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c > > index 9854a50..2c27f6f 100644 > > --- a/drivers/gpu/drm/drm_plane_helper.c > > +++ b/drivers/gpu/drm/drm_plane_helper.c > > @@ -511,12 +511,10 @@ int drm_plane_helper_commit(struct drm_plane *plane, > > if (plane_funcs->cleanup_fb) > > plane_funcs->cleanup_fb(plane, plane_state); > > out: > > - if (plane_state) { > > - if (plane->funcs->atomic_destroy_state) > > - plane->funcs->atomic_destroy_state(plane, plane_state); > > - else > > - drm_atomic_helper_plane_destroy_state(plane, plane_state); > > - } > > + if (plane->funcs->atomic_destroy_state) > > + plane->funcs->atomic_destroy_state(plane, plane_state); > > + else > > + drm_atomic_helper_plane_destroy_state(plane, plane_state); > > Hmm. If plane->state was NULL, then we could swap that with plane_state, > and thus plane_state could become NULL. But that would actually oops in > drm_atomic_plane_disabling() already, so yeah, no way this could work. > > My only concern is the buggy drm_atomic_helper_plane_reset() which can't > fail gracefully if the kmalloc fails. But failure that would probably > lead to explosions all over anyway. > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> I pushed this one to drm-misc-next. Thanks for the patch. > > > > > > return ret; > > } > > -- > > 2.5.0 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel OTC > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c index 9854a50..2c27f6f 100644 --- a/drivers/gpu/drm/drm_plane_helper.c +++ b/drivers/gpu/drm/drm_plane_helper.c @@ -511,12 +511,10 @@ int drm_plane_helper_commit(struct drm_plane *plane, if (plane_funcs->cleanup_fb) plane_funcs->cleanup_fb(plane, plane_state); out: - if (plane_state) { - if (plane->funcs->atomic_destroy_state) - plane->funcs->atomic_destroy_state(plane, plane_state); - else - drm_atomic_helper_plane_destroy_state(plane, plane_state); - } + if (plane->funcs->atomic_destroy_state) + plane->funcs->atomic_destroy_state(plane, plane_state); + else + drm_atomic_helper_plane_destroy_state(plane, plane_state); return ret; }
plane_state can't be NULL anywhere in this function, so the NULL check at the end is redundant, remove it. Cc: dri-devel@lists.freedesktop.org Signed-off-by: Imre Deak <imre.deak@intel.com> --- drivers/gpu/drm/drm_plane_helper.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-)