| Message ID | 20180716230314.3527-1-carsten.behling@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Tuesday 17 July 2018 04:33 AM, Carsten Behling wrote: > modesetting X11 driver may provide negative x/y cordinates in > mdp5_crtc_cursor_move call when rotation is enabled. > > Cursor buffer can overlap down to its negative width/height. > > ROI has to be recalculated for negative x/y indicating using the > lower/right corner of the cursor buffer and hotspot must be set > in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X. Thanks for the patch. Could you tell how to reproduce this issue on a db410c? I was playing with xrandr's --rotate and --reflect options to get a rotated output, but wasn't able to generate negative x/y co-ordinates. I'm using linaro's debian userspace, running lxqt. Thanks, Archit > > Signed-off-by: Carsten Behling <carsten.behling@gmail.com> > --- > Changes in v2: > - fixed format specifier in debug message > > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 51 ++++++++++++++++++++++++++----- > 1 file changed, 43 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c > index 10271359789e..a7f4a6688fec 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c > @@ -65,7 +65,7 @@ struct mdp5_crtc { > struct drm_gem_object *scanout_bo; > uint64_t iova; > uint32_t width, height; > - uint32_t x, y; > + int x, y; > } cursor; > }; > #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base) > @@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, uint32_t *roi_w, uint32_t *roi_h) > * Cursor Region Of Interest (ROI) is a plane read from cursor > * buffer to render. The ROI region is determined by the visibility of > * the cursor point. In the default Cursor image the cursor point will > - * be at the top left of the cursor image, unless it is specified > - * otherwise using hotspot feature. > + * be at the top left of the cursor image. > * > + * Without rotation: > * If the cursor point reaches the right (xres - x < cursor.width) or > * bottom (yres - y < cursor.height) boundary of the screen, then ROI > * width and ROI height need to be evaluated to crop the cursor image > * accordingly. > * (xres-x) will be new cursor width when x > (xres - cursor.width) > * (yres-y) will be new cursor height when y > (yres - cursor.height) > + * > + * With rotation: > + * We get negative x and/or y coordinates. > + * (cursor.width - abs(x)) will be new cursor width when x < 0 > + * (cursor.height - abs(y)) will be new cursor width when y < 0 > */ > - *roi_w = min(mdp5_crtc->cursor.width, xres - > + if (mdp5_crtc->cursor.x >= 0) > + *roi_w = min(mdp5_crtc->cursor.width, xres - > mdp5_crtc->cursor.x); > - *roi_h = min(mdp5_crtc->cursor.height, yres - > + else > + *roi_w = mdp5_crtc->cursor.width - abs(mdp5_crtc->cursor.x); > + if (mdp5_crtc->cursor.y >= 0) > + *roi_h = min(mdp5_crtc->cursor.height, yres - > mdp5_crtc->cursor.y); > + else > + *roi_h = mdp5_crtc->cursor.height - abs(mdp5_crtc->cursor.y); > } > > static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) > @@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) > struct mdp5_kms *mdp5_kms = get_kms(crtc); > const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL; > uint32_t blendcfg, stride; > - uint32_t x, y, width, height; > + uint32_t x, y, src_x, src_y, width, height; > uint32_t roi_w, roi_h; > int lm; > > @@ -800,6 +811,26 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) > > get_roi(crtc, &roi_w, &roi_h); > > + /* If cusror buffer overlaps due to rotation on the > + * upper or left screen border the pixel offset inside > + * the cursor buffer of the ROI is the positive overlap > + * distance. > + */ > + if (mdp5_crtc->cursor.x < 0) { > + src_x = abs(mdp5_crtc->cursor.x); > + x = 0; > + } else { > + src_x = 0; > + } > + if (mdp5_crtc->cursor.y < 0) { > + src_y = abs(mdp5_crtc->cursor.y); > + y = 0; > + } else { > + src_y = 0; > + } > + DBG("%s: x=%u, y=%u roi_w=%u roi_h=%u src_x=%u src_y=%u", > + crtc->name, x, y, roi_w, roi_h, src_x, src_y); > + > mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride); > mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm), > MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB8888)); > @@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) > mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm), > MDP5_LM_CURSOR_START_XY_Y_START(y) | > MDP5_LM_CURSOR_START_XY_X_START(x)); > + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm), > + MDP5_LM_CURSOR_XY_SRC_Y(src_y) | > + MDP5_LM_CURSOR_XY_SRC_X(src_x)); > mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), > mdp5_crtc->cursor.iova); > > @@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) > if (unlikely(!crtc->state->enable)) > return 0; > > - mdp5_crtc->cursor.x = x = max(x, 0); > - mdp5_crtc->cursor.y = y = max(y, 0); > + /* accept negative x/y coordinates up to maximum cursor overlap */ > + mdp5_crtc->cursor.x = x = max(x, -(int)mdp5_crtc->cursor.width); > + mdp5_crtc->cursor.y = y = max(y, -(int)mdp5_crtc->cursor.height); > > get_roi(crtc, &roi_w, &roi_h); > >
Hi, > Thanks for the patch. Could you tell how to reproduce this issue > on a db410c? > > I was playing with xrandr's --rotate and --reflect options to get > a rotated output, but wasn't able to generate negative x/y > co-ordinates. I'm using linaro's debian userspace, running lxqt. I used Yocto Rocko from 96Boards https://github.com/96boards/oe-rpb-manifest/tree/rocko MACHINE=dragonboard-410c DISTRO=rpb rpb-desktop-image Connect HDMI monitor and USB mouse, then 1.) Just boot. Wait for X-Server up. 2.) From my serial console: DISPLAY=:0.0 xrandr -o 2 3.) Try to move the mouse to the upper (the rotated lower) border. Interesting to know that your debian user space is ok. The yocto X11 configuration is very basic. There may be a X11 configuration or extension that does the trick on Debian. Therefore, I asked the X11 people where to fix: https://www.spinics.net/lists/xorg/msg58969.html Best regards -Carsten 2018-07-24 19:33 GMT+02:00 Archit Taneja <architt@codeaurora.org>: > Hi, > > On Tuesday 17 July 2018 04:33 AM, Carsten Behling wrote: > >> modesetting X11 driver may provide negative x/y cordinates in >> mdp5_crtc_cursor_move call when rotation is enabled. >> >> Cursor buffer can overlap down to its negative width/height. >> >> ROI has to be recalculated for negative x/y indicating using the >> lower/right corner of the cursor buffer and hotspot must be set >> in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X. >> > > Thanks for the patch. Could you tell how to reproduce this issue > on a db410c? > > I was playing with xrandr's --rotate and --reflect options to get > a rotated output, but wasn't able to generate negative x/y > co-ordinates. I'm using linaro's debian userspace, running lxqt. > > Thanks, > Archit > > > >> Signed-off-by: Carsten Behling <carsten.behling@gmail.com> >> --- >> Changes in v2: >> - fixed format specifier in debug message >> >> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 51 >> ++++++++++++++++++++++++++----- >> 1 file changed, 43 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c >> b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c >> index 10271359789e..a7f4a6688fec 100644 >> --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c >> +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c >> @@ -65,7 +65,7 @@ struct mdp5_crtc { >> struct drm_gem_object *scanout_bo; >> uint64_t iova; >> uint32_t width, height; >> - uint32_t x, y; >> + int x, y; >> } cursor; >> }; >> #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base) >> @@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, uint32_t >> *roi_w, uint32_t *roi_h) >> * Cursor Region Of Interest (ROI) is a plane read from cursor >> * buffer to render. The ROI region is determined by the >> visibility of >> * the cursor point. In the default Cursor image the cursor point >> will >> - * be at the top left of the cursor image, unless it is specified >> - * otherwise using hotspot feature. >> + * be at the top left of the cursor image. >> * >> + * Without rotation: >> * If the cursor point reaches the right (xres - x < >> cursor.width) or >> * bottom (yres - y < cursor.height) boundary of the screen, then >> ROI >> * width and ROI height need to be evaluated to crop the cursor >> image >> * accordingly. >> * (xres-x) will be new cursor width when x > (xres - >> cursor.width) >> * (yres-y) will be new cursor height when y > (yres - >> cursor.height) >> + * >> + * With rotation: >> + * We get negative x and/or y coordinates. >> + * (cursor.width - abs(x)) will be new cursor width when x < 0 >> + * (cursor.height - abs(y)) will be new cursor width when y < 0 >> */ >> - *roi_w = min(mdp5_crtc->cursor.width, xres - >> + if (mdp5_crtc->cursor.x >= 0) >> + *roi_w = min(mdp5_crtc->cursor.width, xres - >> mdp5_crtc->cursor.x); >> - *roi_h = min(mdp5_crtc->cursor.height, yres - >> + else >> + *roi_w = mdp5_crtc->cursor.width - >> abs(mdp5_crtc->cursor.x); >> + if (mdp5_crtc->cursor.y >= 0) >> + *roi_h = min(mdp5_crtc->cursor.height, yres - >> mdp5_crtc->cursor.y); >> + else >> + *roi_h = mdp5_crtc->cursor.height - >> abs(mdp5_crtc->cursor.y); >> } >> static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) >> @@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc >> *crtc) >> struct mdp5_kms *mdp5_kms = get_kms(crtc); >> const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL; >> uint32_t blendcfg, stride; >> - uint32_t x, y, width, height; >> + uint32_t x, y, src_x, src_y, width, height; >> uint32_t roi_w, roi_h; >> int lm; >> @@ -800,6 +811,26 @@ static void mdp5_crtc_restore_cursor(struct >> drm_crtc *crtc) >> get_roi(crtc, &roi_w, &roi_h); >> + /* If cusror buffer overlaps due to rotation on the >> + * upper or left screen border the pixel offset inside >> + * the cursor buffer of the ROI is the positive overlap >> + * distance. >> + */ >> + if (mdp5_crtc->cursor.x < 0) { >> + src_x = abs(mdp5_crtc->cursor.x); >> + x = 0; >> + } else { >> + src_x = 0; >> + } >> + if (mdp5_crtc->cursor.y < 0) { >> + src_y = abs(mdp5_crtc->cursor.y); >> + y = 0; >> + } else { >> + src_y = 0; >> + } >> + DBG("%s: x=%u, y=%u roi_w=%u roi_h=%u src_x=%u src_y=%u", >> + crtc->name, x, y, roi_w, roi_h, src_x, src_y); >> + >> mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride); >> mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm), >> MDP5_LM_CURSOR_FORMAT_FORMAT(C >> URSOR_FMT_ARGB8888)); >> @@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc >> *crtc) >> mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm), >> MDP5_LM_CURSOR_START_XY_Y_START(y) | >> MDP5_LM_CURSOR_START_XY_X_START(x)); >> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm), >> + MDP5_LM_CURSOR_XY_SRC_Y(src_y) | >> + MDP5_LM_CURSOR_XY_SRC_X(src_x)); >> mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), >> mdp5_crtc->cursor.iova); >> @@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct drm_crtc >> *crtc, int x, int y) >> if (unlikely(!crtc->state->enable)) >> return 0; >> - mdp5_crtc->cursor.x = x = max(x, 0); >> - mdp5_crtc->cursor.y = y = max(y, 0); >> + /* accept negative x/y coordinates up to maximum cursor overlap */ >> + mdp5_crtc->cursor.x = x = max(x, -(int)mdp5_crtc->cursor.width); >> + mdp5_crtc->cursor.y = y = max(y, -(int)mdp5_crtc->cursor.height); >> get_roi(crtc, &roi_w, &roi_h); >> >> > <div dir="ltr"><div><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Hi,</span></div><div><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><br></span></div> <span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> Thanks for the patch. Could you tell how to reproduce this issue</span><br style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial"><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> on a db410c?</span><br style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial">><br style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial"><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> I was playing with xrandr's --rotate and --reflect options to get</span><br style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial"><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> a rotated output, but wasn't able to generate negative x/y</span><br style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial"><span style="font-size:12.8px;text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> co-ordinates. I'm using linaro's debian userspace, running lxqt.</span> <br><div><br></div><div>I used Yocto Rocko from 96Boards</div><div><br></div><div><a href="https://github.com/96boards/oe-rpb-manifest/tree/rocko">https://github.com/96boards/oe-rpb-manifest/tree/rocko</a><br></div><div><br></div><div>MACHINE=dragonboard-410c</div><div>DISTRO=rpb</div><div><br></div><div>rpb-desktop-image</div><div><br></div><div>Connect HDMI monitor and USB mouse, then</div><div><br></div><div>1.) Just boot. Wait for X-Server up.</div><div>2.) From my serial console:</div><div> DISPLAY=:0.0 xrandr -o 2</div><div>3.) Try to move the mouse to the upper (the rotated lower) border.</div><div><br></div><div>Interesting to know that your debian user space is ok. The yocto X11 configuration is very basic.</div><div>There may be a X11 configuration or extension that does the trick on Debian.</div><div><br></div><div>Therefore, I asked the X11 people where to fix:</div><div><br></div><div><a href="https://www.spinics.net/lists/xorg/msg58969.html">https://www.spinics.net/lists/xorg/msg58969.html</a></div><div><br></div><div>Best regards</div><div>-Carsten</div><div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">2018-07-24 19:33 GMT+02:00 Archit Taneja <span dir="ltr"><<a href="mailto:architt@codeaurora.org" target="_blank">architt@codeaurora.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<span class=""><br> <br> On Tuesday 17 July 2018 04:33 AM, Carsten Behling wrote:<br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> modesetting X11 driver may provide negative x/y cordinates in<br> mdp5_crtc_cursor_move call when rotation is enabled.<br> <br> Cursor buffer can overlap down to its negative width/height.<br> <br> ROI has to be recalculated for negative x/y indicating using the<br> lower/right corner of the cursor buffer and hotspot must be set<br> in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X.<br> </blockquote> <br></span> Thanks for the patch. Could you tell how to reproduce this issue<br> on a db410c?<br> <br> I was playing with xrandr's --rotate and --reflect options to get<br> a rotated output, but wasn't able to generate negative x/y<br> co-ordinates. I'm using linaro's debian userspace, running lxqt.<br> <br> Thanks,<br> Archit<div class="HOEnZb"><div class="h5"><br> <br> <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <br> Signed-off-by: Carsten Behling <<a href="mailto:carsten.behling@gmail.com" target="_blank">carsten.behling@gmail.com</a>><br> ---<br> Changes in v2:<br> - fixed format specifier in debug message<br> <br> drivers/gpu/drm/msm/disp/mdp5/<wbr>mdp5_crtc.c | 51 ++++++++++++++++++++++++++----<wbr>-<br> 1 file changed, 43 insertions(+), 8 deletions(-)<br> <br> diff --git a/drivers/gpu/drm/msm/disp/mdp<wbr>5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp<wbr>5/mdp5_crtc.c<br> index 10271359789e..a7f4a6688fec 100644<br> --- a/drivers/gpu/drm/msm/disp/mdp<wbr>5/mdp5_crtc.c<br> +++ b/drivers/gpu/drm/msm/disp/mdp<wbr>5/mdp5_crtc.c<br> @@ -65,7 +65,7 @@ struct mdp5_crtc {<br> struct drm_gem_object *scanout_bo;<br> uint64_t iova;<br> uint32_t width, height;<br> - uint32_t x, y;<br> + int x, y;<br> } cursor;<br> };<br> #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base)<br> @@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, uint32_t *roi_w, uint32_t *roi_h)<br> * Cursor Region Of Interest (ROI) is a plane read from cursor<br> * buffer to render. The ROI region is determined by the visibility of<br> * the cursor point. In the default Cursor image the cursor point will<br> - * be at the top left of the cursor image, unless it is specified<br> - * otherwise using hotspot feature.<br> + * be at the top left of the cursor image.<br> *<br> + * Without rotation:<br> * If the cursor point reaches the right (xres - x < cursor.width) or<br> * bottom (yres - y < cursor.height) boundary of the screen, then ROI<br> * width and ROI height need to be evaluated to crop the cursor image<br> * accordingly.<br> * (xres-x) will be new cursor width when x > (xres - cursor.width)<br> * (yres-y) will be new cursor height when y > (yres - cursor.height)<br> + *<br> + * With rotation:<br> + * We get negative x and/or y coordinates.<br> + * (cursor.width - abs(x)) will be new cursor width when x < 0<br> + * (cursor.height - abs(y)) will be new cursor width when y < 0<br> */<br> - *roi_w = min(mdp5_crtc->cursor.width, xres -<br> + if (mdp5_crtc->cursor.x >= 0)<br> + *roi_w = min(mdp5_crtc->cursor.width, xres -<br> mdp5_crtc->cursor.x);<br> - *roi_h = min(mdp5_crtc->cursor.height, yres -<br> + else<br> + *roi_w = mdp5_crtc->cursor.width - abs(mdp5_crtc->cursor.x);<br> + if (mdp5_crtc->cursor.y >= 0)<br> + *roi_h = min(mdp5_crtc->cursor.height, yres -<br> mdp5_crtc->cursor.y);<br> + else<br> + *roi_h = mdp5_crtc->cursor.height - abs(mdp5_crtc->cursor.y);<br> }<br> static void mdp5_crtc_restore_cursor(struc<wbr>t drm_crtc *crtc)<br> @@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struc<wbr>t drm_crtc *crtc)<br> struct mdp5_kms *mdp5_kms = get_kms(crtc);<br> const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL;<br> uint32_t blendcfg, stride;<br> - uint32_t x, y, width, height;<br> + uint32_t x, y, src_x, src_y, width, height;<br> uint32_t roi_w, roi_h;<br> int lm;<br> @@ -800,6 +811,26 @@ static void mdp5_crtc_restore_cursor(struc<wbr>t drm_crtc *crtc)<br> get_roi(crtc, &roi_w, &roi_h);<br> + /* If cusror buffer overlaps due to rotation on the<br> + * upper or left screen border the pixel offset inside<br> + * the cursor buffer of the ROI is the positive overlap<br> + * distance.<br> + */<br> + if (mdp5_crtc->cursor.x < 0) {<br> + src_x = abs(mdp5_crtc->cursor.x);<br> + x = 0;<br> + } else {<br> + src_x = 0;<br> + }<br> + if (mdp5_crtc->cursor.y < 0) {<br> + src_y = abs(mdp5_crtc->cursor.y);<br> + y = 0;<br> + } else {<br> + src_y = 0;<br> + }<br> + DBG("%s: x=%u, y=%u roi_w=%u roi_h=%u src_x=%u src_y=%u",<br> + crtc->name, x, y, roi_w, roi_h, src_x, src_y);<br> +<br> mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride);<br> mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm),<br> MDP5_LM_CURSOR_FORMAT_FORMAT(C<wbr>URSOR_FMT_ARGB8888));<br> @@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struc<wbr>t drm_crtc *crtc)<br> mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm<wbr>),<br> MDP5_LM_CURSOR_START_XY_Y_STAR<wbr>T(y) |<br> MDP5_LM_CURSOR_START_XY_X_STAR<wbr>T(x));<br> + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm),<br> + MDP5_LM_CURSOR_XY_SRC_Y(src_<wbr>y) |<br> + MDP5_LM_CURSOR_XY_SRC_X(src_<wbr>x));<br> mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(l<wbr>m),<br> mdp5_crtc->cursor.iova);<br> @@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)<br> if (unlikely(!crtc->state->enable<wbr>))<br> return 0;<br> - mdp5_crtc->cursor.x = x = max(x, 0);<br> - mdp5_crtc->cursor.y = y = max(y, 0);<br> + /* accept negative x/y coordinates up to maximum cursor overlap */<br> + mdp5_crtc->cursor.x = x = max(x, -(int)mdp5_crtc->cursor.width)<wbr>;<br> + mdp5_crtc->cursor.y = y = max(y, -(int)mdp5_crtc->cursor.height<wbr>);<br> get_roi(crtc, &roi_w, &roi_h);<br> <br> </blockquote> </div></div></blockquote></div><br></div>
On Wednesday 25 July 2018 08:40 PM, Carsten Behling wrote: > Hi, > >> Thanks for the patch. Could you tell how to reproduce this issue >> on a db410c? > > >> I was playing with xrandr's --rotate and --reflect options to get >> a rotated output, but wasn't able to generate negative x/y >> co-ordinates. I'm using linaro's debian userspace, running lxqt. > > I used Yocto Rocko from 96Boards > > https://github.com/96boards/oe-rpb-manifest/tree/rocko > > MACHINE=dragonboard-410c > DISTRO=rpb > > rpb-desktop-image > > Connect HDMI monitor and USB mouse, then > > 1.) Just boot. Wait for X-Server up. > 2.) From my serial console: > DISPLAY=:0.0 xrandr -o 2 > 3.) Try to move the mouse to the upper (the rotated lower) border. > > Interesting to know that your debian user space is ok. The yocto X11 > configuration is very basic. > There may be a X11 configuration or extension that does the trick on Debian. Thanks, I'll give this a try. The patch looks good, anyway. Rob's queued it for msm-next. Archit > > Therefore, I asked the X11 people where to fix: > > https://www.spinics.net/lists/xorg/msg58969.html > > Best regards > -Carsten > > > 2018-07-24 19:33 GMT+02:00 Archit Taneja <architt@codeaurora.org > <mailto:architt@codeaurora.org>>: > > Hi, > > On Tuesday 17 July 2018 04:33 AM, Carsten Behling wrote: > > modesetting X11 driver may provide negative x/y cordinates in > mdp5_crtc_cursor_move call when rotation is enabled. > > Cursor buffer can overlap down to its negative width/height. > > ROI has to be recalculated for negative x/y indicating using the > lower/right corner of the cursor buffer and hotspot must be set > in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X. > > > Thanks for the patch. Could you tell how to reproduce this issue > on a db410c? > > I was playing with xrandr's --rotate and --reflect options to get > a rotated output, but wasn't able to generate negative x/y > co-ordinates. I'm using linaro's debian userspace, running lxqt. > > Thanks, > Archit > > > > Signed-off-by: Carsten Behling <carsten.behling@gmail.com > <mailto:carsten.behling@gmail.com>> > --- > Changes in v2: > - fixed format specifier in debug message > > drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 51 > ++++++++++++++++++++++++++----- > 1 file changed, 43 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c > index 10271359789e..a7f4a6688fec 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c > @@ -65,7 +65,7 @@ struct mdp5_crtc { > struct drm_gem_object *scanout_bo; > uint64_t iova; > uint32_t width, height; > - uint32_t x, y; > + int x, y; > } cursor; > }; > #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base) > @@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, > uint32_t *roi_w, uint32_t *roi_h) > * Cursor Region Of Interest (ROI) is a plane read from > cursor > * buffer to render. The ROI region is determined by > the visibility of > * the cursor point. In the default Cursor image the > cursor point will > - * be at the top left of the cursor image, unless it is > specified > - * otherwise using hotspot feature. > + * be at the top left of the cursor image. > * > + * Without rotation: > * If the cursor point reaches the right (xres - x < > cursor.width) or > * bottom (yres - y < cursor.height) boundary of the > screen, then ROI > * width and ROI height need to be evaluated to crop > the cursor image > * accordingly. > * (xres-x) will be new cursor width when x > (xres - > cursor.width) > * (yres-y) will be new cursor height when y > (yres - > cursor.height) > + * > + * With rotation: > + * We get negative x and/or y coordinates. > + * (cursor.width - abs(x)) will be new cursor width when > x < 0 > + * (cursor.height - abs(y)) will be new cursor width > when y < 0 > */ > - *roi_w = min(mdp5_crtc->cursor.width, xres - > + if (mdp5_crtc->cursor.x >= 0) > + *roi_w = min(mdp5_crtc->cursor.width, xres - > mdp5_crtc->cursor.x); > - *roi_h = min(mdp5_crtc->cursor.height, yres - > + else > + *roi_w = mdp5_crtc->cursor.width - > abs(mdp5_crtc->cursor.x); > + if (mdp5_crtc->cursor.y >= 0) > + *roi_h = min(mdp5_crtc->cursor.height, yres - > mdp5_crtc->cursor.y); > + else > + *roi_h = mdp5_crtc->cursor.height - > abs(mdp5_crtc->cursor.y); > } > static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) > @@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struct > drm_crtc *crtc) > struct mdp5_kms *mdp5_kms = get_kms(crtc); > const enum mdp5_cursor_alpha cur_alpha = > CURSOR_ALPHA_PER_PIXEL; > uint32_t blendcfg, stride; > - uint32_t x, y, width, height; > + uint32_t x, y, src_x, src_y, width, height; > uint32_t roi_w, roi_h; > int lm; > @@ -800,6 +811,26 @@ static void > mdp5_crtc_restore_cursor(struct drm_crtc *crtc) > get_roi(crtc, &roi_w, &roi_h); > + /* If cusror buffer overlaps due to rotation on the > + * upper or left screen border the pixel offset inside > + * the cursor buffer of the ROI is the positive overlap > + * distance. > + */ > + if (mdp5_crtc->cursor.x < 0) { > + src_x = abs(mdp5_crtc->cursor.x); > + x = 0; > + } else { > + src_x = 0; > + } > + if (mdp5_crtc->cursor.y < 0) { > + src_y = abs(mdp5_crtc->cursor.y); > + y = 0; > + } else { > + src_y = 0; > + } > + DBG("%s: x=%u, y=%u roi_w=%u roi_h=%u src_x=%u src_y=%u", > + crtc->name, x, y, roi_w, roi_h, src_x, src_y); > + > mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), > stride); > mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm), > > MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB8888)); > @@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struct > drm_crtc *crtc) > mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm), > MDP5_LM_CURSOR_START_XY_Y_START(y) | > MDP5_LM_CURSOR_START_XY_X_START(x)); > + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm), > + MDP5_LM_CURSOR_XY_SRC_Y(src_y) | > + MDP5_LM_CURSOR_XY_SRC_X(src_x)); > mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), > mdp5_crtc->cursor.iova); > @@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct > drm_crtc *crtc, int x, int y) > if (unlikely(!crtc->state->enable)) > return 0; > - mdp5_crtc->cursor.x = x = max(x, 0); > - mdp5_crtc->cursor.y = y = max(y, 0); > + /* accept negative x/y coordinates up to maximum cursor > overlap */ > + mdp5_crtc->cursor.x = x = max(x, > -(int)mdp5_crtc->cursor.width); > + mdp5_crtc->cursor.y = y = max(y, > -(int)mdp5_crtc->cursor.height); > get_roi(crtc, &roi_w, &roi_h); > >
diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c index 10271359789e..a7f4a6688fec 100644 --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c @@ -65,7 +65,7 @@ struct mdp5_crtc { struct drm_gem_object *scanout_bo; uint64_t iova; uint32_t width, height; - uint32_t x, y; + int x, y; } cursor; }; #define to_mdp5_crtc(x) container_of(x, struct mdp5_crtc, base) @@ -760,20 +760,31 @@ static void get_roi(struct drm_crtc *crtc, uint32_t *roi_w, uint32_t *roi_h) * Cursor Region Of Interest (ROI) is a plane read from cursor * buffer to render. The ROI region is determined by the visibility of * the cursor point. In the default Cursor image the cursor point will - * be at the top left of the cursor image, unless it is specified - * otherwise using hotspot feature. + * be at the top left of the cursor image. * + * Without rotation: * If the cursor point reaches the right (xres - x < cursor.width) or * bottom (yres - y < cursor.height) boundary of the screen, then ROI * width and ROI height need to be evaluated to crop the cursor image * accordingly. * (xres-x) will be new cursor width when x > (xres - cursor.width) * (yres-y) will be new cursor height when y > (yres - cursor.height) + * + * With rotation: + * We get negative x and/or y coordinates. + * (cursor.width - abs(x)) will be new cursor width when x < 0 + * (cursor.height - abs(y)) will be new cursor width when y < 0 */ - *roi_w = min(mdp5_crtc->cursor.width, xres - + if (mdp5_crtc->cursor.x >= 0) + *roi_w = min(mdp5_crtc->cursor.width, xres - mdp5_crtc->cursor.x); - *roi_h = min(mdp5_crtc->cursor.height, yres - + else + *roi_w = mdp5_crtc->cursor.width - abs(mdp5_crtc->cursor.x); + if (mdp5_crtc->cursor.y >= 0) + *roi_h = min(mdp5_crtc->cursor.height, yres - mdp5_crtc->cursor.y); + else + *roi_h = mdp5_crtc->cursor.height - abs(mdp5_crtc->cursor.y); } static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) @@ -783,7 +794,7 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) struct mdp5_kms *mdp5_kms = get_kms(crtc); const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL; uint32_t blendcfg, stride; - uint32_t x, y, width, height; + uint32_t x, y, src_x, src_y, width, height; uint32_t roi_w, roi_h; int lm; @@ -800,6 +811,26 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) get_roi(crtc, &roi_w, &roi_h); + /* If cusror buffer overlaps due to rotation on the + * upper or left screen border the pixel offset inside + * the cursor buffer of the ROI is the positive overlap + * distance. + */ + if (mdp5_crtc->cursor.x < 0) { + src_x = abs(mdp5_crtc->cursor.x); + x = 0; + } else { + src_x = 0; + } + if (mdp5_crtc->cursor.y < 0) { + src_y = abs(mdp5_crtc->cursor.y); + y = 0; + } else { + src_y = 0; + } + DBG("%s: x=%u, y=%u roi_w=%u roi_h=%u src_x=%u src_y=%u", + crtc->name, x, y, roi_w, roi_h, src_x, src_y); + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride); mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm), MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB8888)); @@ -812,6 +843,9 @@ static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc) mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm), MDP5_LM_CURSOR_START_XY_Y_START(y) | MDP5_LM_CURSOR_START_XY_X_START(x)); + mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm), + MDP5_LM_CURSOR_XY_SRC_Y(src_y) | + MDP5_LM_CURSOR_XY_SRC_X(src_x)); mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm), mdp5_crtc->cursor.iova); @@ -932,8 +966,9 @@ static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) if (unlikely(!crtc->state->enable)) return 0; - mdp5_crtc->cursor.x = x = max(x, 0); - mdp5_crtc->cursor.y = y = max(y, 0); + /* accept negative x/y coordinates up to maximum cursor overlap */ + mdp5_crtc->cursor.x = x = max(x, -(int)mdp5_crtc->cursor.width); + mdp5_crtc->cursor.y = y = max(y, -(int)mdp5_crtc->cursor.height); get_roi(crtc, &roi_w, &roi_h);
modesetting X11 driver may provide negative x/y cordinates in mdp5_crtc_cursor_move call when rotation is enabled. Cursor buffer can overlap down to its negative width/height. ROI has to be recalculated for negative x/y indicating using the lower/right corner of the cursor buffer and hotspot must be set in MDP5_LM_CURSOR_XY_SRC_Y MDP5_LM_CURSOR_XY_SRC_X. Signed-off-by: Carsten Behling <carsten.behling@gmail.com> --- Changes in v2: - fixed format specifier in debug message drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c | 51 ++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 8 deletions(-)