drm/msm/mdp5: negative x/y in cursor move

[Carsten Behling]

I found the solution:

ROI has to be recalculated for negative x/y indicating using the
lower/right corner of the cursor buffer. Further, MDP5_LM_CURSOR_XY_SRC_Y
and MDP5_LM_CURSOR_XY_SRC_X mus be calculated for the hotspot:

Index: kernel-source/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
===================================================================
--- kernel-source.orig/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ kernel-source/drivers/gpu/drm/msm/mdp/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)
@@ -756,10 +756,16 @@ static void get_roi(struct drm_crtc *crt
  * (xres-x) will be new cursor width when x > (xres - cursor.width)
  * (yres-y) will be new cursor height when y > (yres - cursor.height)
  */
- *roi_w = min(mdp5_crtc->cursor.width, xres -
- mdp5_crtc->cursor.x);
- *roi_h = min(mdp5_crtc->cursor.height, yres -
- mdp5_crtc->cursor.y);
+ if (mdp5_crtc->cursor.x >= 0)
+         *roi_w = min(mdp5_crtc->cursor.width, xres -
+         mdp5_crtc->cursor.x);
+ 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)
@@ -769,7 +775,7 @@ static void mdp5_crtc_restore_cursor(str
  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;

@@ -786,6 +792,20 @@ static void mdp5_crtc_restore_cursor(str

  get_roi(crtc, &roi_w, &roi_h);

+        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;
+
+ //printk("x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d\n", 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));
@@ -798,6 +818,9 @@ static void mdp5_crtc_restore_cursor(str
  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);

@@ -903,6 +926,8 @@ static int mdp5_crtc_cursor_move(struct
  uint32_t roi_w;
  uint32_t roi_h;
  unsigned long flags;
+ int border_x = mdp5_crtc->cursor.width * (-1);
+ int border_y = mdp5_crtc->cursor.height * (-1);

  if (!mdp5_crtc->lm_cursor_enabled) {
  dev_warn(dev->dev,
@@ -918,8 +943,8 @@ static int mdp5_crtc_cursor_move(struct
  if (unlikely(!crtc->state->enable))
  return 0;

- mdp5_crtc->cursor.x = x = max(x, 0);
- mdp5_crtc->cursor.y = y = max(y, 0);
+ mdp5_crtc->cursor.x = x = max(x, border_x);
+ mdp5_crtc->cursor.y = y = max(y, border_y);

  get_roi(crtc, &roi_w, &roi_h);

Best regards
-Carsten

2018-07-10 12:11 GMT+02:00 Carsten Behling <carsten.behling at googlemail.com>:

> Hi,
>
> modesetting X11 driver may provide negative x/y cordinates in
> mdp5_crtc_cursor_move(...) call when rotation is enabled.
>
> Because of
>
> static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
> {
> ...
> mdp5_crtc->cursor.x = x = max(x, 0);
> mdp5_crtc->cursor.y = y = max(y, 0);
> ...
> }
>
> x/y is calmped to 0/0 in those cases resulting that the cursor does not
> move anymore beyond mdp5_crtc->cursor.width, mdp5_crtc->cursor.height.
>
> For e.g rotation of 180 degree that means that the upper left cursor point
> stays never reaches the region (0/0) to  (mdp5_crtc->cursor.width/
> mdp5_crtc->cursor.height).
>
> I already asked the X men if this should be fixed in modesetting driver or
> in the kernel CRT
> functions:
>
> https://www.spinics.net/lists/xorg/msg58969.html
>
> They told me to fix this in the kernel.
>
> So, I suppose:
>
> 1.) cursor x should be rather clamped instead to
>
> static int mdp5_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) {
> ...
> mdp5_crtc->cursor.x = x = max(x, -mdp5_crtc->cursor.width);
> mdp5_crtc->cursor.y = y = max(y, -mdp5_crtc->cursor.height);
> ...
> }
>
>  2.) The ROI calculation must be extendet to:
>
> static void get_roi(struct drm_crtc *crtc, uint32_t *roi_w, uint32_t
> *roi_h)
> {
> ...
> if (x>=0)
>         *roi_w = min(mdp5_crtc->cursor.width, xres -
>         mdp5_crtc->cursor.x);
> else
>         *roi_w = mdp5_crtc->cursor.width - abs(mdp5_crtc->cursor.x);
> if (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);
> ...
> }
>
> 3.) There has to be some kind of hotspot setup
> in mdp5_crtc_restore_cursor(...)
>
> Since I have no MDP5 documentation, I don't know how to setup the hotspot
> and I can't
> implement 3.)
>
> Please help!
>
> Best regards
> -Carsten
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20180710/8fccb64b/attachment-0001.html>