On Wed, Apr 13, 2016 at 10:52:34AM +0800, Ying Liu wrote:
On Wed, Apr 13, 2016 at 12:00 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Apr 05, 2016 at 04:50:39PM +0800, Liu Ying wrote:
Transitional drivers might access the NULL pointer plane->state in drm_helper_crtc_mode_set_base(), which causes NULL pointer dereference. So, let's reset it before handing it over to those drivers. commit e4f31ad2b713 ("drm: reset empty state in transitional helpers") did the same thing for other transitional helpers, but it seems this one was missed.
Signed-off-by: Liu Ying gnuiyl@gmail.com
This is kinda intentionally not done, assuming that drivers (when transitioning) can't handle the full state stuff yet.
It's a question whether the assumption is correct or not. IMHO, during the transition, the old/new state door has been opened to drivers.
I'm not entirely opposed to this, but then we should do it for both plane and crtc states, and in all cases I think.
It looks this doesn't hurt.
Completely new drivers should never use transitional helpers, but instead just bring up using native atomic helpers directly. So what exactly do you need this for?
This is needed for my WIP imx-drm transitional plane driver. It would access the fb pointer of the old plane state(the empty plane state in question) to do atomic check. Also, ->atomic_update() will do something like page flip if the fb pointer is not NULL(meaning that the plane was enabled before the update). All of this aims to make the driver behave the similar way after the transition.
If you need this in your driver, just make sure you're calling ->reset hooks yourself as needed? Also this really is just for transitioning, in the end no driver should permanently end up using these functions. In the end transitional helpers here are meant as guidelines/crutches, every driver is slightly different. Adding hacks to make things work smoothly for all of them is impossible, and imo better to just move real fast to proper atomic. -Daniel
Regards, Liu Ying
-Daniel
drivers/gpu/drm/drm_crtc_helper.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c index 79555d2..66ca313 100644 --- a/drivers/gpu/drm/drm_crtc_helper.c +++ b/drivers/gpu/drm/drm_crtc_helper.c @@ -1053,10 +1053,12 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
if (plane->funcs->atomic_duplicate_state) plane_state = plane->funcs->atomic_duplicate_state(plane);
else if (plane->state)
else {if (!plane->state)drm_atomic_helper_plane_reset(plane);plane_state = drm_atomic_helper_plane_duplicate_state(plane);
elseplane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL);
} if (!plane_state) return -ENOMEM; plane_state->plane = plane;-- 2.5.0
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch