* [PATCH] drm/atmel-hlcdc: prevent divide by zero
@ 2019-01-08 12:31 Peter Rosin
2019-01-09 10:12 ` Daniel Vetter
0 siblings, 1 reply; 5+ messages in thread
From: Peter Rosin @ 2019-01-08 12:31 UTC (permalink / raw)
To: linux-kernel
Cc: Peter Rosin, Boris Brezillon, David Airlie, Nicolas Ferre,
Alexandre Belloni, dri-devel, linux-arm-kernel
While trying to temporarily hide a plane, one thing that was attempted
was to call (from libdrm)
drmModeSetPlane(fd, plane_id, crtc_id, fb_id, 0,
0, 0, 0, 0, 0, 0, 0, 0);
This call causes a pair of "Division by zero in kernel." messages. Kill
those messages, but preserve the current behaviour that also happen to
make the plane disappear with the above call.
Signed-off-by: Peter Rosin <peda@axentia.se>
---
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
Side note, when comparing with drm_atomic_helper_check_plane_state(), I get
the feeling that the src rect should be clipped together with the crtc rect
if/when clipping is needed. That function calls drm_rect_clip_scaled(), and
the equivalent does not seem to happen here. Should clipping be performed
on the src rect?
Cheers,
Peter
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
index 3cc489b870fe..1bdb30dc218c 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
@@ -675,10 +675,16 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
state->crtc_y = 0;
}
- patched_src_w = DIV_ROUND_CLOSEST(patched_crtc_w * state->src_w,
- state->crtc_w);
- patched_src_h = DIV_ROUND_CLOSEST(patched_crtc_h * state->src_h,
- state->crtc_h);
+ if (state->crtc_w)
+ patched_src_w = DIV_ROUND_CLOSEST(patched_crtc_w * state->src_w,
+ state->crtc_w);
+ else
+ patched_src_w = 0;
+ if (state->crtc_h)
+ patched_src_h = DIV_ROUND_CLOSEST(patched_crtc_h * state->src_h,
+ state->crtc_h);
+ else
+ patched_src_h = 0;
hsub = drm_format_horz_chroma_subsampling(fb->format->format);
vsub = drm_format_vert_chroma_subsampling(fb->format->format);
--
2.11.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/atmel-hlcdc: prevent divide by zero
2019-01-08 12:31 [PATCH] drm/atmel-hlcdc: prevent divide by zero Peter Rosin
@ 2019-01-09 10:12 ` Daniel Vetter
2019-01-09 11:37 ` Peter Rosin
2019-01-09 11:37 ` Boris Brezillon
0 siblings, 2 replies; 5+ messages in thread
From: Daniel Vetter @ 2019-01-09 10:12 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Alexandre Belloni, David Airlie, Nicolas Ferre,
dri-devel, Boris Brezillon, linux-arm-kernel
On Tue, Jan 08, 2019 at 12:31:36PM +0000, Peter Rosin wrote:
> While trying to temporarily hide a plane, one thing that was attempted
> was to call (from libdrm)
>
> drmModeSetPlane(fd, plane_id, crtc_id, fb_id, 0,
> 0, 0, 0, 0, 0, 0, 0, 0);
>
> This call causes a pair of "Division by zero in kernel." messages. Kill
> those messages, but preserve the current behaviour that also happen to
> make the plane disappear with the above call.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> Side note, when comparing with drm_atomic_helper_check_plane_state(), I get
> the feeling that the src rect should be clipped together with the crtc rect
> if/when clipping is needed. That function calls drm_rect_clip_scaled(), and
> the equivalent does not seem to happen here. Should clipping be performed
> on the src rect?
Any reasons atmel can't switch over to that helper? Would compute a nice
->visible state variable for you ...
-Daniel
>
> Cheers,
> Peter
>
> diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> index 3cc489b870fe..1bdb30dc218c 100644
> --- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> +++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c
> @@ -675,10 +675,16 @@ static int atmel_hlcdc_plane_atomic_check(struct drm_plane *p,
> state->crtc_y = 0;
> }
>
> - patched_src_w = DIV_ROUND_CLOSEST(patched_crtc_w * state->src_w,
> - state->crtc_w);
> - patched_src_h = DIV_ROUND_CLOSEST(patched_crtc_h * state->src_h,
> - state->crtc_h);
> + if (state->crtc_w)
> + patched_src_w = DIV_ROUND_CLOSEST(patched_crtc_w * state->src_w,
> + state->crtc_w);
> + else
> + patched_src_w = 0;
> + if (state->crtc_h)
> + patched_src_h = DIV_ROUND_CLOSEST(patched_crtc_h * state->src_h,
> + state->crtc_h);
> + else
> + patched_src_h = 0;
>
> hsub = drm_format_horz_chroma_subsampling(fb->format->format);
> vsub = drm_format_vert_chroma_subsampling(fb->format->format);
> --
> 2.11.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/atmel-hlcdc: prevent divide by zero
2019-01-09 10:12 ` Daniel Vetter
@ 2019-01-09 11:37 ` Peter Rosin
2019-01-09 12:00 ` Boris Brezillon
2019-01-09 11:37 ` Boris Brezillon
1 sibling, 1 reply; 5+ messages in thread
From: Peter Rosin @ 2019-01-09 11:37 UTC (permalink / raw)
To: linux-kernel, Alexandre Belloni, David Airlie, Nicolas Ferre,
dri-devel, Boris Brezillon, linux-arm-kernel
On 2019-01-09 11:12, Daniel Vetter wrote:
> On Tue, Jan 08, 2019 at 12:31:36PM +0000, Peter Rosin wrote:
>> While trying to temporarily hide a plane, one thing that was attempted
>> was to call (from libdrm)
>>
>> drmModeSetPlane(fd, plane_id, crtc_id, fb_id, 0,
>> 0, 0, 0, 0, 0, 0, 0, 0);
>>
>> This call causes a pair of "Division by zero in kernel." messages. Kill
>> those messages, but preserve the current behaviour that also happen to
>> make the plane disappear with the above call.
>>
>> Signed-off-by: Peter Rosin <peda@axentia.se>
>> ---
>> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 14 ++++++++++----
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> Side note, when comparing with drm_atomic_helper_check_plane_state(), I get
>> the feeling that the src rect should be clipped together with the crtc rect
>> if/when clipping is needed. That function calls drm_rect_clip_scaled(), and
>> the equivalent does not seem to happen here. Should clipping be performed
>> on the src rect?
>
> Any reasons atmel can't switch over to that helper? Would compute a nice
> ->visible state variable for you ...
> -Daniel
I have not researched that, and as stated above, I was not sure if that was the
correct approach to begin with. Boris, any insights? Does this code predate the
helper or something?
Maybe there's some register bit that hides a plane explicitly, matching the
->visible state variable? I haven't looked at that either...
Cheers,
Peter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/atmel-hlcdc: prevent divide by zero
2019-01-09 10:12 ` Daniel Vetter
2019-01-09 11:37 ` Peter Rosin
@ 2019-01-09 11:37 ` Boris Brezillon
1 sibling, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2019-01-09 11:37 UTC (permalink / raw)
To: Daniel Vetter
Cc: Peter Rosin, Alexandre Belloni, David Airlie, linux-kernel,
dri-devel, Nicolas Ferre, Boris Brezillon, linux-arm-kernel
On Wed, 9 Jan 2019 11:12:24 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Jan 08, 2019 at 12:31:36PM +0000, Peter Rosin wrote:
> > While trying to temporarily hide a plane, one thing that was attempted
> > was to call (from libdrm)
> >
> > drmModeSetPlane(fd, plane_id, crtc_id, fb_id, 0,
> > 0, 0, 0, 0, 0, 0, 0, 0);
> >
> > This call causes a pair of "Division by zero in kernel." messages. Kill
> > those messages, but preserve the current behaviour that also happen to
> > make the plane disappear with the above call.
> >
> > Signed-off-by: Peter Rosin <peda@axentia.se>
> > ---
> > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 14 ++++++++++----
> > 1 file changed, 10 insertions(+), 4 deletions(-)
> >
> > Side note, when comparing with drm_atomic_helper_check_plane_state(), I get
> > the feeling that the src rect should be clipped together with the crtc rect
> > if/when clipping is needed. That function calls drm_rect_clip_scaled(), and
> > the equivalent does not seem to happen here. Should clipping be performed
> > on the src rect?
>
> Any reasons atmel can't switch over to that helper? Would compute a nice
> ->visible state variable for you ...
We should definitely use drm_atomic_helper_check_plane_state().
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] drm/atmel-hlcdc: prevent divide by zero
2019-01-09 11:37 ` Peter Rosin
@ 2019-01-09 12:00 ` Boris Brezillon
0 siblings, 0 replies; 5+ messages in thread
From: Boris Brezillon @ 2019-01-09 12:00 UTC (permalink / raw)
To: Peter Rosin
Cc: linux-kernel, Alexandre Belloni, David Airlie, Nicolas Ferre,
dri-devel, Boris Brezillon, linux-arm-kernel
On Wed, 9 Jan 2019 11:37:19 +0000
Peter Rosin <peda@axentia.se> wrote:
> On 2019-01-09 11:12, Daniel Vetter wrote:
> > On Tue, Jan 08, 2019 at 12:31:36PM +0000, Peter Rosin wrote:
> >> While trying to temporarily hide a plane, one thing that was attempted
> >> was to call (from libdrm)
> >>
> >> drmModeSetPlane(fd, plane_id, crtc_id, fb_id, 0,
> >> 0, 0, 0, 0, 0, 0, 0, 0);
> >>
> >> This call causes a pair of "Division by zero in kernel." messages. Kill
> >> those messages, but preserve the current behaviour that also happen to
> >> make the plane disappear with the above call.
> >>
> >> Signed-off-by: Peter Rosin <peda@axentia.se>
> >> ---
> >> drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_plane.c | 14 ++++++++++----
> >> 1 file changed, 10 insertions(+), 4 deletions(-)
> >>
> >> Side note, when comparing with drm_atomic_helper_check_plane_state(), I get
> >> the feeling that the src rect should be clipped together with the crtc rect
> >> if/when clipping is needed. That function calls drm_rect_clip_scaled(), and
> >> the equivalent does not seem to happen here. Should clipping be performed
> >> on the src rect?
> >
> > Any reasons atmel can't switch over to that helper? Would compute a nice
> > ->visible state variable for you ...
> > -Daniel
>
> I have not researched that, and as stated above, I was not sure if that was the
> correct approach to begin with. Boris, any insights?
You can look at vc4_plane.c if you want an example of how this helper
can be used to get clipped dimensions of the plane.
> Does this code predate the
> helper or something?
Yes, and I must admit I'm not actively maintaining the driver, so there
are probably a few other things we could simplify by using generic
helpers.
>
> Maybe there's some register bit that hides a plane explicitly, matching the
> ->visible state variable? I haven't looked at that either...
I don't think so, but you can simply disable the plane when ->visible
is false.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-09 12:00 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-08 12:31 [PATCH] drm/atmel-hlcdc: prevent divide by zero Peter Rosin
2019-01-09 10:12 ` Daniel Vetter
2019-01-09 11:37 ` Peter Rosin
2019-01-09 12:00 ` Boris Brezillon
2019-01-09 11:37 ` Boris Brezillon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).