* [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
@ 2015-12-01 8:15 Dongsheng Wang
2015-12-24 5:28 ` 答复: " Meng Yi
2015-12-24 13:37 ` Daniel Stone
0 siblings, 2 replies; 9+ messages in thread
From: Dongsheng Wang @ 2015-12-01 8:15 UTC (permalink / raw)
To: airlied; +Cc: dri-devel, Yi Meng
From: Jianwei Wang <jianwei.wang.chn@gmail.com>
For state->fb may be NULL in fsl_dcu_drm_plane_atomic_check function,
if so, return -EINVAL. No need check in fsl_dcu_drm_plane_atomic_update
anymore.
Signed-off-by: Jianwei Wang <jianwei.wang.chn@gmail.com>
Signed-off-by: Yi Meng <b56799@freescale.com>
Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
Tested-by: Stefan Agner <stefan@agner.ch>
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 51daaea..a8932a8 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -41,6 +41,9 @@ static int fsl_dcu_drm_plane_atomic_check(struct drm_plane *plane,
{
struct drm_framebuffer *fb = state->fb;
+ if (!fb)
+ return -EINVAL;
+
switch (fb->pixel_format) {
case DRM_FORMAT_RGB565:
case DRM_FORMAT_RGB888:
@@ -85,9 +88,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
unsigned int alpha, bpp;
int index, ret;
- if (!fb)
- return;
-
index = fsl_dcu_drm_plane_index(plane);
if (index < 0)
return;
--
2.1.0.27.g96db324
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* 答复: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
2015-12-01 8:15 [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug Dongsheng Wang
@ 2015-12-24 5:28 ` Meng Yi
2015-12-27 3:54 ` Stefan Agner
2015-12-24 13:37 ` Daniel Stone
1 sibling, 1 reply; 9+ messages in thread
From: Meng Yi @ 2015-12-24 5:28 UTC (permalink / raw)
To: Dongsheng Wang, airlied; +Cc: dri-devel, Yi Meng-B56799
Tested-by: Meng Yi <meng.yi@nxp.com>
-----邮件原件-----
发件人: Dongsheng Wang [mailto:Dongsheng.Wang@freescale.com]
发送时间: Tuesday, December 01, 2015 4:16 PM
收件人: airlied@linux.ie
抄送: stefan@agner.ch; dri-devel@lists.freedesktop.org; Jianwei Wang <jianwei.wang.chn@gmail.com>; Yi Meng-B56799 <B56799@freescale.com>; Wang Dongsheng-B40534 <Dongsheng.Wang@freescale.com>
主题: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
From: Jianwei Wang <jianwei.wang.chn@gmail.com>
For state->fb may be NULL in fsl_dcu_drm_plane_atomic_check function, if so, return -EINVAL. No need check in fsl_dcu_drm_plane_atomic_update anymore.
Signed-off-by: Jianwei Wang <jianwei.wang.chn@gmail.com>
Signed-off-by: Yi Meng <b56799@freescale.com>
Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
Tested-by: Stefan Agner <stefan@agner.ch>
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
index 51daaea..a8932a8 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
@@ -41,6 +41,9 @@ static int fsl_dcu_drm_plane_atomic_check(struct drm_plane *plane, {
struct drm_framebuffer *fb = state->fb;
+ if (!fb)
+ return -EINVAL;
+
switch (fb->pixel_format) {
case DRM_FORMAT_RGB565:
case DRM_FORMAT_RGB888:
@@ -85,9 +88,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct drm_plane *plane,
unsigned int alpha, bpp;
int index, ret;
- if (!fb)
- return;
-
index = fsl_dcu_drm_plane_index(plane);
if (index < 0)
return;
--
2.1.0.27.g96db324
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
2015-12-01 8:15 [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug Dongsheng Wang
2015-12-24 5:28 ` 答复: " Meng Yi
@ 2015-12-24 13:37 ` Daniel Stone
1 sibling, 0 replies; 9+ messages in thread
From: Daniel Stone @ 2015-12-24 13:37 UTC (permalink / raw)
To: Dongsheng Wang; +Cc: Yi Meng, dri-devel
Hi,
On 1 December 2015 at 08:15, Dongsheng Wang
<dongsheng.wang@freescale.com> wrote:
> From: Jianwei Wang <jianwei.wang.chn@gmail.com>
>
> For state->fb may be NULL in fsl_dcu_drm_plane_atomic_check function,
> if so, return -EINVAL. No need check in fsl_dcu_drm_plane_atomic_update
> anymore.
>
> Signed-off-by: Jianwei Wang <jianwei.wang.chn@gmail.com>
> Signed-off-by: Yi Meng <b56799@freescale.com>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> Tested-by: Stefan Agner <stefan@agner.ch>
>
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> index 51daaea..a8932a8 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> @@ -41,6 +41,9 @@ static int fsl_dcu_drm_plane_atomic_check(struct drm_plane *plane,
> {
> struct drm_framebuffer *fb = state->fb;
>
> + if (!fb)
> + return -EINVAL;
This should be return 0 instead, else you can never disable a plane ...
Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 答复: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
2015-12-24 5:28 ` 答复: " Meng Yi
@ 2015-12-27 3:54 ` Stefan Agner
2015-12-28 15:14 ` Daniel Stone
2015-12-30 7:37 ` 答复: " Meng Yi
0 siblings, 2 replies; 9+ messages in thread
From: Stefan Agner @ 2015-12-27 3:54 UTC (permalink / raw)
To: Meng Yi; +Cc: dri-devel, Yi Meng-B56799
Hi Meng,
The only situation I could observe that is when fsl,panel was not
valid...
However, I think with my patch "drm/fsl-dcu: Fix no fb check bug", this
situation can be avoided completely:
https://lkml.org/lkml/2015/11/18/950
With that patch applied, and a non-existing panel assigned, the DRM
driver already fails at probe time gracefully:
[ 0.488291] [drm] Initialized drm 1.1.0 20060810
[ 0.501576] fsl-dcu 40058000.dcu: failed to initialize mode setting
So I think that a state->fb check for NULL is not necessary at all...
Can you test my patch to see if it fixes the issue for you too?
--
Stefan
On 2015-12-23 21:28, Meng Yi wrote:
> Tested-by: Meng Yi <meng.yi@nxp.com>
>
> -----邮件原件-----
> 发件人: Dongsheng Wang [mailto:Dongsheng.Wang@freescale.com]
> 发送时间: Tuesday, December 01, 2015 4:16 PM
> 收件人: airlied@linux.ie
> 抄送: stefan@agner.ch; dri-devel@lists.freedesktop.org; Jianwei Wang
> <jianwei.wang.chn@gmail.com>; Yi Meng-B56799 <B56799@freescale.com>;
> Wang Dongsheng-B40534 <Dongsheng.Wang@freescale.com>
> 主题: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
>
> From: Jianwei Wang <jianwei.wang.chn@gmail.com>
>
> For state->fb may be NULL in fsl_dcu_drm_plane_atomic_check function,
> if so, return -EINVAL. No need check in
> fsl_dcu_drm_plane_atomic_update anymore.
>
> Signed-off-by: Jianwei Wang <jianwei.wang.chn@gmail.com>
> Signed-off-by: Yi Meng <b56799@freescale.com>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> Tested-by: Stefan Agner <stefan@agner.ch>
>
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> index 51daaea..a8932a8 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> @@ -41,6 +41,9 @@ static int fsl_dcu_drm_plane_atomic_check(struct
> drm_plane *plane, {
> struct drm_framebuffer *fb = state->fb;
>
> + if (!fb)
> + return -EINVAL;
> +
> switch (fb->pixel_format) {
> case DRM_FORMAT_RGB565:
> case DRM_FORMAT_RGB888:
> @@ -85,9 +88,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct
> drm_plane *plane,
> unsigned int alpha, bpp;
> int index, ret;
>
> - if (!fb)
> - return;
> -
> index = fsl_dcu_drm_plane_index(plane);
> if (index < 0)
> return;
> --
> 2.1.0.27.g96db324
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
2015-12-27 3:54 ` Stefan Agner
@ 2015-12-28 15:14 ` Daniel Stone
2015-12-30 7:37 ` 答复: " Meng Yi
1 sibling, 0 replies; 9+ messages in thread
From: Daniel Stone @ 2015-12-28 15:14 UTC (permalink / raw)
To: Stefan Agner; +Cc: Meng Yi, Yi Meng-B56799, dri-devel
[-- Attachment #1.1: Type: text/plain, Size: 722 bytes --]
Hi Stefan,
On Sunday, 27 December 2015, Stefan Agner <stefan@agner.ch> wrote:
>
> However, I think with my patch "drm/fsl-dcu: Fix no fb check bug", this
> situation can be avoided completely:
> https://lkml.org/lkml/2015/11/18/950
>
> With that patch applied, and a non-existing panel assigned, the DRM
> driver already fails at probe time gracefully:
> [ 0.488291] [drm] Initialized drm 1.1.0 20060810
> [ 0.501576] fsl-dcu 40058000.dcu: failed to initialize mode setting
>
> So I think that a state->fb check for NULL is not necessary at all...
>
> Can you test my patch to see if it fixes the issue for you too?
>
This is still required; state->fb being NULL means the plane is being
disabled.
Cheers,
Daniel
[-- Attachment #1.2: Type: text/html, Size: 1071 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: 答复: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
2015-12-27 3:54 ` Stefan Agner
2015-12-28 15:14 ` Daniel Stone
@ 2015-12-30 7:37 ` Meng Yi
2016-01-01 15:10 ` Daniel Stone
1 sibling, 1 reply; 9+ messages in thread
From: Meng Yi @ 2015-12-30 7:37 UTC (permalink / raw)
To: Stefan Agner; +Cc: dri-devel, Yi Meng-B56799
Hi, Stefan
I have tested your patch, It seems good to me.
But I think state->fb check is still necessary, because fb is related to crtc , and panel is related to connector,. When fsl,panel is not valid, it indicate that connector is not available, but fb check is still needed. But I am not so sure, and what do you think?
--
Meng Yi
-----Original Message-----
From: Stefan Agner [mailto:stefan@agner.ch]
Sent: Sunday, December 27, 2015 11:54 AM
To: Meng Yi <meng.yi@nxp.com>
Cc: Dongsheng Wang <Dongsheng.Wang@freescale.com>; airlied@linux.ie; dri-devel@lists.freedesktop.org; Jianwei Wang <jianwei.wang.chn@gmail.com>; Yi Meng-B56799 <B56799@freescale.com>
Subject: Re: 答复: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
Hi Meng,
The only situation I could observe that is when fsl,panel was not valid...
However, I think with my patch "drm/fsl-dcu: Fix no fb check bug", this situation can be avoided completely:
https://lkml.org/lkml/2015/11/18/950
With that patch applied, and a non-existing panel assigned, the DRM driver already fails at probe time gracefully:
[ 0.488291] [drm] Initialized drm 1.1.0 20060810
[ 0.501576] fsl-dcu 40058000.dcu: failed to initialize mode setting
So I think that a state->fb check for NULL is not necessary at all...
Can you test my patch to see if it fixes the issue for you too?
--
Stefan
On 2015-12-23 21:28, Meng Yi wrote:
> Tested-by: Meng Yi <meng.yi@nxp.com>
>
> -----邮件原件-----
> 发件人: Dongsheng Wang [mailto:Dongsheng.Wang@freescale.com]
> 发送时间: Tuesday, December 01, 2015 4:16 PM
> 收件人: airlied@linux.ie
> 抄送: stefan@agner.ch; dri-devel@lists.freedesktop.org; Jianwei Wang
> <jianwei.wang.chn@gmail.com>; Yi Meng-B56799 <B56799@freescale.com>;
> Wang Dongsheng-B40534 <Dongsheng.Wang@freescale.com>
> 主题: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
>
> From: Jianwei Wang <jianwei.wang.chn@gmail.com>
>
> For state->fb may be NULL in fsl_dcu_drm_plane_atomic_check function,
> if so, return -EINVAL. No need check in
> fsl_dcu_drm_plane_atomic_update anymore.
>
> Signed-off-by: Jianwei Wang <jianwei.wang.chn@gmail.com>
> Signed-off-by: Yi Meng <b56799@freescale.com>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@freescale.com>
> Tested-by: Stefan Agner <stefan@agner.ch>
>
> diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> index 51daaea..a8932a8 100644
> --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c
> @@ -41,6 +41,9 @@ static int fsl_dcu_drm_plane_atomic_check(struct
> drm_plane *plane, {
> struct drm_framebuffer *fb = state->fb;
>
> + if (!fb)
> + return -EINVAL;
> +
> switch (fb->pixel_format) {
> case DRM_FORMAT_RGB565:
> case DRM_FORMAT_RGB888:
> @@ -85,9 +88,6 @@ static void fsl_dcu_drm_plane_atomic_update(struct
> drm_plane *plane,
> unsigned int alpha, bpp;
> int index, ret;
>
> - if (!fb)
> - return;
> -
> index = fsl_dcu_drm_plane_index(plane);
> if (index < 0)
> return;
> --
> 2.1.0.27.g96db324
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 答复: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
2015-12-30 7:37 ` 答复: " Meng Yi
@ 2016-01-01 15:10 ` Daniel Stone
2016-01-04 6:07 ` Stefan Agner
2016-01-04 6:48 ` Meng Yi
0 siblings, 2 replies; 9+ messages in thread
From: Daniel Stone @ 2016-01-01 15:10 UTC (permalink / raw)
To: Meng Yi; +Cc: dri-devel, Yi Meng-B56799
Hi,
On 30 December 2015 at 07:37, Meng Yi <meng.yi@nxp.com> wrote:
> I have tested your patch, It seems good to me.
> But I think state->fb check is still necessary, because fb is related to crtc , and panel is related to connector,. When fsl,panel is not valid, it indicate that connector is not available, but fb check is still needed. But I am not so sure, and what do you think?
fb is tied to planes. Planes are tied to CRTCs. CRTCs are tied to
connectors. Connectors are tied to the panel. If the panel is not
found, no connectors will be activated, so no CRTCs will be enabled,
so no planes will be enabled.
Please let me be very clear though: setting fb == NULL is legitimate.
Userspace may do this at any time. Crashing when plane->state->fb ==
NULL is an error in your driver and must be fixed.
Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: 答复: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
2016-01-01 15:10 ` Daniel Stone
@ 2016-01-04 6:07 ` Stefan Agner
2016-01-04 6:48 ` Meng Yi
1 sibling, 0 replies; 9+ messages in thread
From: Stefan Agner @ 2016-01-04 6:07 UTC (permalink / raw)
To: Daniel Stone; +Cc: Meng Yi, Yi Meng-B56799, dri-devel
On 2016-01-01 07:10, Daniel Stone wrote:
> Hi,
>
> On 30 December 2015 at 07:37, Meng Yi <meng.yi@nxp.com> wrote:
>> I have tested your patch, It seems good to me.
>> But I think state->fb check is still necessary, because fb is related to crtc , and panel is related to connector,. When fsl,panel is not valid, it indicate that connector is not available, but fb check is still needed. But I am not so sure, and what do you think?
>
> fb is tied to planes. Planes are tied to CRTCs. CRTCs are tied to
> connectors. Connectors are tied to the panel. If the panel is not
> found, no connectors will be activated, so no CRTCs will be enabled,
> so no planes will be enabled.
>
> Please let me be very clear though: setting fb == NULL is legitimate.
> Userspace may do this at any time. Crashing when plane->state->fb ==
> NULL is an error in your driver and must be fixed.
Thanks for this clarification. Ok, I agree the patch is needed despite
my fix then...
Acked-by: Stefan Agner <stefan@agner.ch>
--
Stefan
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: 答复: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
2016-01-01 15:10 ` Daniel Stone
2016-01-04 6:07 ` Stefan Agner
@ 2016-01-04 6:48 ` Meng Yi
1 sibling, 0 replies; 9+ messages in thread
From: Meng Yi @ 2016-01-04 6:48 UTC (permalink / raw)
To: Daniel Stone; +Cc: dri-devel
Hi, Daniel
Thanks for your clarification, and I will fix this error.
--
Meng
-----Original Message-----
From: Daniel Stone [mailto:daniel@fooishbar.org]
Sent: Friday, January 01, 2016 11:10 PM
To: Meng Yi <meng.yi@nxp.com>
Cc: Stefan Agner <stefan@agner.ch>; dri-devel@lists.freedesktop.org; Yi Meng-B56799 <B56799@freescale.com>
Subject: Re: 答复: [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug
Hi,
On 30 December 2015 at 07:37, Meng Yi <meng.yi@nxp.com> wrote:
> I have tested your patch, It seems good to me.
> But I think state->fb check is still necessary, because fb is related to crtc , and panel is related to connector,. When fsl,panel is not valid, it indicate that connector is not available, but fb check is still needed. But I am not so sure, and what do you think?
fb is tied to planes. Planes are tied to CRTCs. CRTCs are tied to connectors. Connectors are tied to the panel. If the panel is not found, no connectors will be activated, so no CRTCs will be enabled, so no planes will be enabled.
Please let me be very clear though: setting fb == NULL is legitimate.
Userspace may do this at any time. Crashing when plane->state->fb == NULL is an error in your driver and must be fixed.
Cheers,
Daniel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-01-04 6:48 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-01 8:15 [RESEND 1/3] drm: fsl-dcu: Fix no fb check bug Dongsheng Wang
2015-12-24 5:28 ` 答复: " Meng Yi
2015-12-27 3:54 ` Stefan Agner
2015-12-28 15:14 ` Daniel Stone
2015-12-30 7:37 ` 答复: " Meng Yi
2016-01-01 15:10 ` Daniel Stone
2016-01-04 6:07 ` Stefan Agner
2016-01-04 6:48 ` Meng Yi
2015-12-24 13:37 ` Daniel Stone
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.