All of lore.kernel.org
 help / color / mirror / Atom feed
From: ville.syrjala@linux.intel.com (Ville Syrjälä)
To: linus-amlogic@lists.infradead.org
Subject: [PATCH] drm/meson: Fix plane atomic check when no crtc for the plane
Date: Wed, 4 Jan 2017 15:27:37 +0200	[thread overview]
Message-ID: <20170104132737.GF31595@intel.com> (raw)
In-Reply-To: <1483435254-27955-1-git-send-email-narmstrong@baylibre.com>

On Tue, Jan 03, 2017 at 10:20:54AM +0100, Neil Armstrong wrote:
> When no CRTC is associated with the plane, the meson_plane_atomic_check()
> call breaks the kernel with an Oops.
> 
> Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_plane.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 4942ca0..7890e30 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -51,6 +51,9 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_rect clip = { 0, };
>  
> +	if (!state->crtc)
> +		return 0;
> +

Since you're not going to call drm_plane_helper_check_state() you will
fail to update plane_state->visible.

What i915 does in this case is look for state->crtc first, and if that's
NULL it'll pick the crtc from the old state (plane->state->crtc). It
looks a bit ugly, but maybe we should hide it in a small helper function
that could be used by all drivers?

>  	crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
>  	if (IS_ERR(crtc_state))
>  		return PTR_ERR(crtc_state);
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrj?l?
Intel OTC

WARNING: multiple messages have this Message-ID (diff)
From: ville.syrjala@linux.intel.com (Ville Syrjälä)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] drm/meson: Fix plane atomic check when no crtc for the plane
Date: Wed, 4 Jan 2017 15:27:37 +0200	[thread overview]
Message-ID: <20170104132737.GF31595@intel.com> (raw)
In-Reply-To: <1483435254-27955-1-git-send-email-narmstrong@baylibre.com>

On Tue, Jan 03, 2017 at 10:20:54AM +0100, Neil Armstrong wrote:
> When no CRTC is associated with the plane, the meson_plane_atomic_check()
> call breaks the kernel with an Oops.
> 
> Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_plane.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 4942ca0..7890e30 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -51,6 +51,9 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_rect clip = { 0, };
>  
> +	if (!state->crtc)
> +		return 0;
> +

Since you're not going to call drm_plane_helper_check_state() you will
fail to update plane_state->visible.

What i915 does in this case is look for state->crtc first, and if that's
NULL it'll pick the crtc from the old state (plane->state->crtc). It
looks a bit ugly, but maybe we should hide it in a small helper function
that could be used by all drivers?

>  	crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
>  	if (IS_ERR(crtc_state))
>  		return PTR_ERR(crtc_state);
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrj?l?
Intel OTC

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: linux-amlogic@lists.infradead.org,
	dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH] drm/meson: Fix plane atomic check when no crtc for the plane
Date: Wed, 4 Jan 2017 15:27:37 +0200	[thread overview]
Message-ID: <20170104132737.GF31595@intel.com> (raw)
In-Reply-To: <1483435254-27955-1-git-send-email-narmstrong@baylibre.com>

On Tue, Jan 03, 2017 at 10:20:54AM +0100, Neil Armstrong wrote:
> When no CRTC is associated with the plane, the meson_plane_atomic_check()
> call breaks the kernel with an Oops.
> 
> Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_plane.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 4942ca0..7890e30 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -51,6 +51,9 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_rect clip = { 0, };
>  
> +	if (!state->crtc)
> +		return 0;
> +

Since you're not going to call drm_plane_helper_check_state() you will
fail to update plane_state->visible.

What i915 does in this case is look for state->crtc first, and if that's
NULL it'll pick the crtc from the old state (plane->state->crtc). It
looks a bit ugly, but maybe we should hide it in a small helper function
that could be used by all drivers?

>  	crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
>  	if (IS_ERR(crtc_state))
>  		return PTR_ERR(crtc_state);
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Neil Armstrong <narmstrong@baylibre.com>
Cc: airlied@linux.ie, linux-amlogic@lists.infradead.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	dri-devel@lists.freedesktop.org
Subject: Re: [PATCH] drm/meson: Fix plane atomic check when no crtc for the plane
Date: Wed, 4 Jan 2017 15:27:37 +0200	[thread overview]
Message-ID: <20170104132737.GF31595@intel.com> (raw)
In-Reply-To: <1483435254-27955-1-git-send-email-narmstrong@baylibre.com>

On Tue, Jan 03, 2017 at 10:20:54AM +0100, Neil Armstrong wrote:
> When no CRTC is associated with the plane, the meson_plane_atomic_check()
> call breaks the kernel with an Oops.
> 
> Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/meson/meson_plane.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 4942ca0..7890e30 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -51,6 +51,9 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
>  	struct drm_crtc_state *crtc_state;
>  	struct drm_rect clip = { 0, };
>  
> +	if (!state->crtc)
> +		return 0;
> +

Since you're not going to call drm_plane_helper_check_state() you will
fail to update plane_state->visible.

What i915 does in this case is look for state->crtc first, and if that's
NULL it'll pick the crtc from the old state (plane->state->crtc). It
looks a bit ugly, but maybe we should hide it in a small helper function
that could be used by all drivers?

>  	crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
>  	if (IS_ERR(crtc_state))
>  		return PTR_ERR(crtc_state);
> -- 
> 1.9.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

  parent reply	other threads:[~2017-01-04 13:27 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-01-03  9:20 [PATCH] drm/meson: Fix plane atomic check when no crtc for the plane Neil Armstrong
2017-01-03  9:20 ` Neil Armstrong
2017-01-03  9:20 ` Neil Armstrong
2017-01-03  9:20 ` Neil Armstrong
2017-01-04  9:10 ` Daniel Vetter
2017-01-04  9:10   ` Daniel Vetter
2017-01-04  9:10   ` Daniel Vetter
2017-01-04  9:10   ` Daniel Vetter
2017-01-04  9:57   ` Neil Armstrong
2017-01-04  9:57     ` Neil Armstrong
2017-01-04  9:57     ` Neil Armstrong
2017-01-04  9:57     ` Neil Armstrong
2017-01-04 13:27 ` Ville Syrjälä [this message]
2017-01-04 13:27   ` Ville Syrjälä
2017-01-04 13:27   ` Ville Syrjälä
2017-01-04 13:27   ` Ville Syrjälä
2017-01-04 13:45   ` Neil Armstrong
2017-01-04 13:45     ` Neil Armstrong
2017-01-04 13:45     ` Neil Armstrong
2017-01-04 13:45     ` Neil Armstrong

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=20170104132737.GF31595@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=linus-amlogic@lists.infradead.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.