Re: [Intel-gfx] [PATCH] drm/i915/display: assume some pixelrate for src smaller than 1
On 11.1.2023 22.09, Drew Davenport wrote:
On Wed, Jan 11, 2023 at 09:39:26PM +0200, Ville Syrjälä wrote:
On Wed, Jan 11, 2023 at 11:28:51AM -0700, Drew Davenport wrote:
On Wed, Jan 11, 2023 at 04:19:00PM +0200, Ville Syrjälä wrote:
On Wed, Jan 04, 2023 at 02:44:48PM +0200, Juha-Pekka Heikkila wrote:
intel_adjusted_rate() didn't take into account src rectangle
can be less than 1 in width or height.
This should not get called in those cases. What does the
backtrace look like?
In my repro of this issue, the backtrace looks as follows:
[ 180.798331] RIP: 0010:intel_plane_pixel_rate+0x4a/0x53
[ 180.798336] Code:
[ 180.798338] RSP: 0018:b080ce4179b8 EFLAGS: 00010246
[ 180.798341] RAX: RBX: 98cd22a24000 RCX: 0a00
[ 180.798343] RDX: RSI: 98cccbae7000 RDI:
[ 180.798346] RBP: b080ce4179b8 R08: 00087780 R09: 0002
[ 180.798348] R10: 0a00 R11: R12:
[ 180.798350] R13: 98cd0e495400 R14: 98ccc34e R15: 98cccbae7000
[ 180.798352] FS: 7b84119b5000() GS:98d02f90()
knlGS:
[ 180.798354] CS: 0010 DS: ES: CR0: 80050033
[ 180.798357] CR2: 7ffc2d5e4080 CR3: 0001042ee006 CR4: 00770ee0
[ 180.798359] PKRU: 5554
[ 180.798361] Call Trace:
[ 180.798364]
[ 180.798366] intel_plane_atomic_check_with_state+0x1fd/0x6ea
[ 180.798370] ? intel_plane_atomic_check+0x11b/0x145
[ 180.798373] intel_atomic_check_planes+0x263/0x7ce
[ 180.798376] ? drm_atomic_helper_check_modeset+0x189/0x923
[ 180.798380] intel_atomic_check+0x14e4/0x184d
[ 180.798382] ? intel_rps_mark_interactive+0x23/0x6a
[ 180.798386] drm_atomic_check_only+0x3ec/0x98f
[ 180.798391] drm_atomic_commit+0xa2/0x105
[ 180.798394] ? drm_atomic_set_fb_for_plane+0x96/0xa5
[ 180.798397] drm_atomic_helper_update_plane+0xdc/0x11f
[ 180.798400] drm_mode_setplane+0x236/0x30c
[ 180.798404] ? drm_any_plane_has_format+0x51/0x51
[ 180.798407] drm_ioctl_kernel+0xda/0x14d
[ 180.798411] drm_ioctl+0x27e/0x3b4
[ 180.798414] ? drm_any_plane_has_format+0x51/0x51
[ 180.798418] __se_sys_ioctl+0x7a/0xbc
[ 180.798421] do_syscall_64+0x55/0x9d
[ 180.798424] ? exit_to_user_mode_prepare+0x3c/0x8b
[ 180.798427] entry_SYSCALL_64_after_hwframe+0x61/0xcb
If this function shouldn't be called in such a case, then perhaps
I should revist my original attempt at fixing this in
https://patchwork.freedesktop.org/patch/516060 by rejecting such a
configuration?
I'm saying that this should be impossible already. At least
I can't immediately see anything that could call this with
an invisible plane.
In my repro case, I called drmModeSetPlane with the src_h parameter set
to 65535 (so the largest 16.16 number that's less than one). This got
through any existing checks on the height of the src rect, resulting in
the divide-by-zero error in intel_plane_pixel_rate.
While investigating this, I tried setting src_h to 0, but this
configuration got rejected somewhere along the line before it got
through the intel_plane_pixel_rate.
Here's one of the igt tests I had come up when debugging this:
---
#include "igt.h"
igt_main
{
igt_subtest_f("testi") {
int drm_fd = drm_open_driver_master(DRIVER_INTEL);
igt_display_t display;
igt_plane_t* plane;
igt_output_t *output;
igt_fb_t fb;
kmstest_set_vt_graphics_mode();
igt_display_require(, drm_fd);
igt_display_require_output();
output = igt_get_single_output_for_pipe(, PIPE_A);
igt_output_set_pipe(output, PIPE_A);
igt_display_commit_atomic(,
DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
igt_create_pattern_fb(drm_fd, 256, 256,
DRM_FORMAT_XRGB,
DRM_FORMAT_MOD_NONE,
);
plane = igt_output_get_plane_type(output,
DRM_PLANE_TYPE_PRIMARY);
igt_plane_set_position(plane, 0, 0);
igt_plane_set_fb(plane, );
igt_plane_set_prop_value(plane, IGT_PLANE_SRC_H, IGT_FIXED(0,
30));
igt_plane_set_size(plane, 256, 8);
igt_display_commit_atomic(,
DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
}
}
---
This should give callstack ending same way as what Drew had seen. With
my patch in place this test will die with einval coming from kernel from
skl_update_scaler(..) where it doesn't pass range checks (and will note
on dmesg about it)
/Juha-Pekka
I'll respond to Alan on that thread.
Signed-off-by: Juha-Pekka Heikkila
---
drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
Re: [Intel-gfx] [PATCH] drm/i915/display: assume some pixelrate for src smaller than 1
On Wed, Jan 11, 2023 at 09:39:26PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 11, 2023 at 11:28:51AM -0700, Drew Davenport wrote:
> > On Wed, Jan 11, 2023 at 04:19:00PM +0200, Ville Syrjälä wrote:
> > > On Wed, Jan 04, 2023 at 02:44:48PM +0200, Juha-Pekka Heikkila wrote:
> > > > intel_adjusted_rate() didn't take into account src rectangle
> > > > can be less than 1 in width or height.
> > >
> > > This should not get called in those cases. What does the
> > > backtrace look like?
> >
> > In my repro of this issue, the backtrace looks as follows:
> >
> > [ 180.798331] RIP: 0010:intel_plane_pixel_rate+0x4a/0x53
> > [ 180.798336] Code:
> > [ 180.798338] RSP: 0018:b080ce4179b8 EFLAGS: 00010246
> > [ 180.798341] RAX: RBX: 98cd22a24000 RCX:
> > 0a00
> > [ 180.798343] RDX: RSI: 98cccbae7000 RDI:
> >
> > [ 180.798346] RBP: b080ce4179b8 R08: 00087780 R09:
> > 0002
> > [ 180.798348] R10: 0a00 R11: R12:
> >
> > [ 180.798350] R13: 98cd0e495400 R14: 98ccc34e R15:
> > 98cccbae7000
> > [ 180.798352] FS: 7b84119b5000() GS:98d02f90()
> > knlGS:
> > [ 180.798354] CS: 0010 DS: ES: CR0: 80050033
> > [ 180.798357] CR2: 7ffc2d5e4080 CR3: 0001042ee006 CR4:
> > 00770ee0
> > [ 180.798359] PKRU: 5554
> > [ 180.798361] Call Trace:
> > [ 180.798364]
> > [ 180.798366] intel_plane_atomic_check_with_state+0x1fd/0x6ea
> > [ 180.798370] ? intel_plane_atomic_check+0x11b/0x145
> > [ 180.798373] intel_atomic_check_planes+0x263/0x7ce
> > [ 180.798376] ? drm_atomic_helper_check_modeset+0x189/0x923
> > [ 180.798380] intel_atomic_check+0x14e4/0x184d
> > [ 180.798382] ? intel_rps_mark_interactive+0x23/0x6a
> > [ 180.798386] drm_atomic_check_only+0x3ec/0x98f
> > [ 180.798391] drm_atomic_commit+0xa2/0x105
> > [ 180.798394] ? drm_atomic_set_fb_for_plane+0x96/0xa5
> > [ 180.798397] drm_atomic_helper_update_plane+0xdc/0x11f
> > [ 180.798400] drm_mode_setplane+0x236/0x30c
> > [ 180.798404] ? drm_any_plane_has_format+0x51/0x51
> > [ 180.798407] drm_ioctl_kernel+0xda/0x14d
> > [ 180.798411] drm_ioctl+0x27e/0x3b4
> > [ 180.798414] ? drm_any_plane_has_format+0x51/0x51
> > [ 180.798418] __se_sys_ioctl+0x7a/0xbc
> > [ 180.798421] do_syscall_64+0x55/0x9d
> > [ 180.798424] ? exit_to_user_mode_prepare+0x3c/0x8b
> > [ 180.798427] entry_SYSCALL_64_after_hwframe+0x61/0xcb
> >
> > If this function shouldn't be called in such a case, then perhaps
> > I should revist my original attempt at fixing this in
> > https://patchwork.freedesktop.org/patch/516060 by rejecting such a
> > configuration?
>
> I'm saying that this should be impossible already. At least
> I can't immediately see anything that could call this with
> an invisible plane.
In my repro case, I called drmModeSetPlane with the src_h parameter set
to 65535 (so the largest 16.16 number that's less than one). This got
through any existing checks on the height of the src rect, resulting in
the divide-by-zero error in intel_plane_pixel_rate.
While investigating this, I tried setting src_h to 0, but this
configuration got rejected somewhere along the line before it got
through the intel_plane_pixel_rate.
>
> >
> > I'll respond to Alan on that thread.
> >
> > >
> > > >
> > > > Signed-off-by: Juha-Pekka Heikkila
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +---
> > > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > index 10e1fc9d0698..a9948e8d3543 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > > @@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct
> > > > drm_rect *src,
> > > > const struct drm_rect *dst,
> > > > unsigned int rate)
> > > > {
> > > > - unsigned int src_w, src_h, dst_w, dst_h;
> > > > + unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
> > > >
> > > > src_w = drm_rect_width(src) >> 16;
> > > > src_h = drm_rect_height(src) >> 16;
> > > > @@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct
> > > > drm_rect *src,
> > > > dst_w = min(src_w, dst_w);
> > > > dst_h = min(src_h, dst_h);
> > > >
> > > > - return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> > > > - dst_w * dst_h);
> > > > + /* in case src contained only fractional part */
> > > > + dst_wh = max(dst_w * dst_h, (unsigned) 1);
> > > > +
> > > > + return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> > > > dst_wh);
> > > > }
> > > >
> > > > unsigned int
Re: [Intel-gfx] [PATCH] drm/i915/display: assume some pixelrate for src smaller than 1
On Wed, Jan 11, 2023 at 11:28:51AM -0700, Drew Davenport wrote:
> On Wed, Jan 11, 2023 at 04:19:00PM +0200, Ville Syrjälä wrote:
> > On Wed, Jan 04, 2023 at 02:44:48PM +0200, Juha-Pekka Heikkila wrote:
> > > intel_adjusted_rate() didn't take into account src rectangle
> > > can be less than 1 in width or height.
> >
> > This should not get called in those cases. What does the
> > backtrace look like?
>
> In my repro of this issue, the backtrace looks as follows:
>
> [ 180.798331] RIP: 0010:intel_plane_pixel_rate+0x4a/0x53
> [ 180.798336] Code:
> [ 180.798338] RSP: 0018:b080ce4179b8 EFLAGS: 00010246
> [ 180.798341] RAX: RBX: 98cd22a24000 RCX:
> 0a00
> [ 180.798343] RDX: RSI: 98cccbae7000 RDI:
>
> [ 180.798346] RBP: b080ce4179b8 R08: 00087780 R09:
> 0002
> [ 180.798348] R10: 0a00 R11: R12:
>
> [ 180.798350] R13: 98cd0e495400 R14: 98ccc34e R15:
> 98cccbae7000
> [ 180.798352] FS: 7b84119b5000() GS:98d02f90()
> knlGS:
> [ 180.798354] CS: 0010 DS: ES: CR0: 80050033
> [ 180.798357] CR2: 7ffc2d5e4080 CR3: 0001042ee006 CR4:
> 00770ee0
> [ 180.798359] PKRU: 5554
> [ 180.798361] Call Trace:
> [ 180.798364]
> [ 180.798366] intel_plane_atomic_check_with_state+0x1fd/0x6ea
> [ 180.798370] ? intel_plane_atomic_check+0x11b/0x145
> [ 180.798373] intel_atomic_check_planes+0x263/0x7ce
> [ 180.798376] ? drm_atomic_helper_check_modeset+0x189/0x923
> [ 180.798380] intel_atomic_check+0x14e4/0x184d
> [ 180.798382] ? intel_rps_mark_interactive+0x23/0x6a
> [ 180.798386] drm_atomic_check_only+0x3ec/0x98f
> [ 180.798391] drm_atomic_commit+0xa2/0x105
> [ 180.798394] ? drm_atomic_set_fb_for_plane+0x96/0xa5
> [ 180.798397] drm_atomic_helper_update_plane+0xdc/0x11f
> [ 180.798400] drm_mode_setplane+0x236/0x30c
> [ 180.798404] ? drm_any_plane_has_format+0x51/0x51
> [ 180.798407] drm_ioctl_kernel+0xda/0x14d
> [ 180.798411] drm_ioctl+0x27e/0x3b4
> [ 180.798414] ? drm_any_plane_has_format+0x51/0x51
> [ 180.798418] __se_sys_ioctl+0x7a/0xbc
> [ 180.798421] do_syscall_64+0x55/0x9d
> [ 180.798424] ? exit_to_user_mode_prepare+0x3c/0x8b
> [ 180.798427] entry_SYSCALL_64_after_hwframe+0x61/0xcb
>
> If this function shouldn't be called in such a case, then perhaps
> I should revist my original attempt at fixing this in
> https://patchwork.freedesktop.org/patch/516060 by rejecting such a
> configuration?
I'm saying that this should be impossible already. At least
I can't immediately see anything that could call this with
an invisible plane.
>
> I'll respond to Alan on that thread.
>
> >
> > >
> > > Signed-off-by: Juha-Pekka Heikkila
> > > ---
> > > drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +---
> > > 1 file changed, 5 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > index 10e1fc9d0698..a9948e8d3543 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > > @@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct
> > > drm_rect *src,
> > >const struct drm_rect *dst,
> > >unsigned int rate)
> > > {
> > > - unsigned int src_w, src_h, dst_w, dst_h;
> > > + unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
> > >
> > > src_w = drm_rect_width(src) >> 16;
> > > src_h = drm_rect_height(src) >> 16;
> > > @@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct
> > > drm_rect *src,
> > > dst_w = min(src_w, dst_w);
> > > dst_h = min(src_h, dst_h);
> > >
> > > - return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> > > - dst_w * dst_h);
> > > + /* in case src contained only fractional part */
> > > + dst_wh = max(dst_w * dst_h, (unsigned) 1);
> > > +
> > > + return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
> > > }
> > >
> > > unsigned int intel_plane_pixel_rate(const struct intel_crtc_state
> > > *crtc_state,
> > > --
> > > 2.37.3
> >
> > --
> > Ville Syrjälä
> > Intel
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH] drm/i915/display: assume some pixelrate for src smaller than 1
On Wed, Jan 11, 2023 at 04:19:00PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 04, 2023 at 02:44:48PM +0200, Juha-Pekka Heikkila wrote:
> > intel_adjusted_rate() didn't take into account src rectangle
> > can be less than 1 in width or height.
>
> This should not get called in those cases. What does the
> backtrace look like?
In my repro of this issue, the backtrace looks as follows:
[ 180.798331] RIP: 0010:intel_plane_pixel_rate+0x4a/0x53
[ 180.798336] Code:
[ 180.798338] RSP: 0018:b080ce4179b8 EFLAGS: 00010246
[ 180.798341] RAX: RBX: 98cd22a24000 RCX: 0a00
[ 180.798343] RDX: RSI: 98cccbae7000 RDI:
[ 180.798346] RBP: b080ce4179b8 R08: 00087780 R09: 0002
[ 180.798348] R10: 0a00 R11: R12:
[ 180.798350] R13: 98cd0e495400 R14: 98ccc34e R15: 98cccbae7000
[ 180.798352] FS: 7b84119b5000() GS:98d02f90()
knlGS:
[ 180.798354] CS: 0010 DS: ES: CR0: 80050033
[ 180.798357] CR2: 7ffc2d5e4080 CR3: 0001042ee006 CR4: 00770ee0
[ 180.798359] PKRU: 5554
[ 180.798361] Call Trace:
[ 180.798364]
[ 180.798366] intel_plane_atomic_check_with_state+0x1fd/0x6ea
[ 180.798370] ? intel_plane_atomic_check+0x11b/0x145
[ 180.798373] intel_atomic_check_planes+0x263/0x7ce
[ 180.798376] ? drm_atomic_helper_check_modeset+0x189/0x923
[ 180.798380] intel_atomic_check+0x14e4/0x184d
[ 180.798382] ? intel_rps_mark_interactive+0x23/0x6a
[ 180.798386] drm_atomic_check_only+0x3ec/0x98f
[ 180.798391] drm_atomic_commit+0xa2/0x105
[ 180.798394] ? drm_atomic_set_fb_for_plane+0x96/0xa5
[ 180.798397] drm_atomic_helper_update_plane+0xdc/0x11f
[ 180.798400] drm_mode_setplane+0x236/0x30c
[ 180.798404] ? drm_any_plane_has_format+0x51/0x51
[ 180.798407] drm_ioctl_kernel+0xda/0x14d
[ 180.798411] drm_ioctl+0x27e/0x3b4
[ 180.798414] ? drm_any_plane_has_format+0x51/0x51
[ 180.798418] __se_sys_ioctl+0x7a/0xbc
[ 180.798421] do_syscall_64+0x55/0x9d
[ 180.798424] ? exit_to_user_mode_prepare+0x3c/0x8b
[ 180.798427] entry_SYSCALL_64_after_hwframe+0x61/0xcb
If this function shouldn't be called in such a case, then perhaps
I should revist my original attempt at fixing this in
https://patchwork.freedesktop.org/patch/516060 by rejecting such a
configuration?
I'll respond to Alan on that thread.
>
> >
> > Signed-off-by: Juha-Pekka Heikkila
> > ---
> > drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +---
> > 1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > index 10e1fc9d0698..a9948e8d3543 100644
> > --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> > @@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> > *src,
> > const struct drm_rect *dst,
> > unsigned int rate)
> > {
> > - unsigned int src_w, src_h, dst_w, dst_h;
> > + unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
> >
> > src_w = drm_rect_width(src) >> 16;
> > src_h = drm_rect_height(src) >> 16;
> > @@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> > *src,
> > dst_w = min(src_w, dst_w);
> > dst_h = min(src_h, dst_h);
> >
> > - return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> > - dst_w * dst_h);
> > + /* in case src contained only fractional part */
> > + dst_wh = max(dst_w * dst_h, (unsigned) 1);
> > +
> > + return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
> > }
> >
> > unsigned int intel_plane_pixel_rate(const struct intel_crtc_state
> > *crtc_state,
> > --
> > 2.37.3
>
> --
> Ville Syrjälä
> Intel
Re: [Intel-gfx] [PATCH] drm/i915/display: assume some pixelrate for src smaller than 1
On Wed, Jan 04, 2023 at 02:44:48PM +0200, Juha-Pekka Heikkila wrote:
> intel_adjusted_rate() didn't take into account src rectangle
> can be less than 1 in width or height.
This should not get called in those cases. What does the
backtrace look like?
>
> Signed-off-by: Juha-Pekka Heikkila
> ---
> drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 10e1fc9d0698..a9948e8d3543 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
>const struct drm_rect *dst,
>unsigned int rate)
> {
> - unsigned int src_w, src_h, dst_w, dst_h;
> + unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
>
> src_w = drm_rect_width(src) >> 16;
> src_h = drm_rect_height(src) >> 16;
> @@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
> dst_w = min(src_w, dst_w);
> dst_h = min(src_h, dst_h);
>
> - return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> - dst_w * dst_h);
> + /* in case src contained only fractional part */
> + dst_wh = max(dst_w * dst_h, (unsigned) 1);
> +
> + return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
> }
>
> unsigned int intel_plane_pixel_rate(const struct intel_crtc_state
> *crtc_state,
> --
> 2.37.3
--
Ville Syrjälä
Intel
Re: [Intel-gfx] [PATCH] drm/i915/display: assume some pixelrate for src smaller than 1
On Wed, 04 Jan 2023, Juha-Pekka Heikkila wrote:
> intel_adjusted_rate() didn't take into account src rectangle
> can be less than 1 in width or height.
>
> Signed-off-by: Juha-Pekka Heikkila
> ---
> drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> index 10e1fc9d0698..a9948e8d3543 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
> @@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
>const struct drm_rect *dst,
>unsigned int rate)
> {
> - unsigned int src_w, src_h, dst_w, dst_h;
> + unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
>
> src_w = drm_rect_width(src) >> 16;
> src_h = drm_rect_height(src) >> 16;
> @@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct drm_rect
> *src,
> dst_w = min(src_w, dst_w);
> dst_h = min(src_h, dst_h);
>
> - return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
> - dst_w * dst_h);
> + /* in case src contained only fractional part */
> + dst_wh = max(dst_w * dst_h, (unsigned) 1);
The options are to use 1U or max_t(), but please don't cast the
parameters of max().
Side note, the explicit "unsigned int" is always preferred over the
implicit "unsigned".
BR,
Jani.
> +
> + return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
> }
>
> unsigned int intel_plane_pixel_rate(const struct intel_crtc_state
> *crtc_state,
--
Jani Nikula, Intel Open Source Graphics Center
[Intel-gfx] [PATCH] drm/i915/display: assume some pixelrate for src smaller than 1
intel_adjusted_rate() didn't take into account src rectangle
can be less than 1 in width or height.
Signed-off-by: Juha-Pekka Heikkila
---
drivers/gpu/drm/i915/display/intel_atomic_plane.c | 8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
index 10e1fc9d0698..a9948e8d3543 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic_plane.c
@@ -144,7 +144,7 @@ unsigned int intel_adjusted_rate(const struct drm_rect *src,
const struct drm_rect *dst,
unsigned int rate)
{
- unsigned int src_w, src_h, dst_w, dst_h;
+ unsigned int src_w, src_h, dst_w, dst_h, dst_wh;
src_w = drm_rect_width(src) >> 16;
src_h = drm_rect_height(src) >> 16;
@@ -155,8 +155,10 @@ unsigned int intel_adjusted_rate(const struct drm_rect
*src,
dst_w = min(src_w, dst_w);
dst_h = min(src_h, dst_h);
- return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h),
- dst_w * dst_h);
+ /* in case src contained only fractional part */
+ dst_wh = max(dst_w * dst_h, (unsigned) 1);
+
+ return DIV_ROUND_UP_ULL(mul_u32_u32(rate, src_w * src_h), dst_wh);
}
unsigned int intel_plane_pixel_rate(const struct intel_crtc_state *crtc_state,
--
2.37.3
