Re: [PATCH v2] drm/msm/display: negative x/y in cursor move

From: Archit Taneja
Date: Thu Jul 26 2018 - 03:25:36 EST



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@xxxxxxxxxxxxxx <mailto:architt@xxxxxxxxxxxxxx>>:

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@xxxxxxxxx
<mailto:carsten.behling@xxxxxxxxx>>
---
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);