From: Daniel Vetter <daniel@ffwll.ch>
To: Liu Ying <gnuiyl@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/crtc_helper: Reset empty plane state in drm_helper_crtc_mode_set_base()
Date: Tue, 12 Apr 2016 18:00:06 +0200 [thread overview]
Message-ID: <20160412160006.GI2510@phenom.ffwll.local> (raw)
In-Reply-To: <1459846239-8946-1-git-send-email-gnuiyl@gmail.com>
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.
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.
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?
-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);
> - else
> - plane_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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2016-04-12 15:59 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-04-05 8:50 [PATCH] drm/crtc_helper: Reset empty plane state in drm_helper_crtc_mode_set_base() Liu Ying
2016-04-12 16:00 ` Daniel Vetter [this message]
2016-04-13 2:52 ` Ying Liu
2016-04-13 10:33 ` Daniel Vetter
2016-04-14 3:25 ` Ying Liu
2016-04-14 6:18 ` Daniel Vetter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20160412160006.GI2510@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=daniel.vetter@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=gnuiyl@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox