All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/msm/display: negative x/y in cursor move
@ 2018-07-16 17:49 ` Carsten Behling
  0 siblings, 0 replies; 11+ messages in thread
From: Carsten Behling @ 2018-07-16 17:49 UTC (permalink / raw)
  Cc: Carsten Behling, Rob Clark, David Airlie, Archit Taneja,
	Sean Paul, Steve Kowalik, Daniel Vetter, Viresh Kumar,
	linux-arm-msm, dri-devel, freedreno, linux-kernel

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>
---
 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..43a86582876c 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=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
+		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);
 
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH] drm/msm/display: negative x/y in cursor move
@ 2018-07-16 17:49 ` Carsten Behling
  0 siblings, 0 replies; 11+ messages in thread
From: Carsten Behling @ 2018-07-16 17:49 UTC (permalink / raw)
  Cc: Carsten Behling, Rob Clark, David Airlie, Archit Taneja,
	Sean Paul, Steve Kowalik, Daniel Vetter, Viresh Kumar,
	linux-arm-msm, dri-devel, freedreno, linux-kernel

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>
---
 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..43a86582876c 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=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
+		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);
 
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/msm/display: negative x/y in cursor move
  2018-07-16 17:49 ` Carsten Behling
@ 2018-07-16 22:06   ` kbuild test robot
  -1 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-07-16 22:06 UTC (permalink / raw)
  To: Carsten Behling
  Cc: Carsten Behling, Steve Kowalik, David Airlie, Daniel Vetter,
	linux-arm-msm, linux-kernel, dri-devel, kbuild-all, Viresh Kumar,
	freedreno

[-- Attachment #1: Type: text/plain, Size: 5858 bytes --]

Hi Carsten,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robclark/msm-next]
[also build test WARNING on v4.18-rc5 next-20180716]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Carsten-Behling/drm-msm-display-negative-x-y-in-cursor-move/20180717-031351
base:   git://people.freedesktop.org/~robclark/linux msm-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

   In file included from include/drm/drm_mm.h:49:0,
                    from include/drm/drmP.h:73,
                    from include/drm/drm_modeset_helper.h:26,
                    from include/drm/drm_crtc_helper.h:44,
                    from drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:22:
   drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c: In function 'mdp5_crtc_restore_cursor':
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:6: warning: format '%s' expects argument of type 'char *', but argument 3 has type 'uint32_t {aka unsigned int}' [-Wformat=]
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
         ^
   include/drm/drm_print.h:285:25: note: in definition of macro 'DRM_DEBUG_DRIVER'
     drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
                            ^~~
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:2: note: in expansion of macro 'DBG'
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
     ^~~
   drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:8: note: format string is defined here
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
          ~^
          %d
   In file included from include/drm/drm_mm.h:49:0,
                    from include/drm/drmP.h:73,
                    from include/drm/drm_modeset_helper.h:26,
                    from include/drm/drm_crtc_helper.h:44,
                    from drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:22:
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:6: warning: format '%d' expects a matching 'int' argument [-Wformat=]
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
         ^
   include/drm/drm_print.h:285:25: note: in definition of macro 'DRM_DEBUG_DRIVER'
     drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
                            ^~~
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:2: note: in expansion of macro 'DBG'
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
     ^~~
   drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:56: note: format string is defined here
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
                                                          ~^

vim +831 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c

   789	
   790	static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
   791	{
   792		struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
   793		struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
   794		struct mdp5_kms *mdp5_kms = get_kms(crtc);
   795		const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL;
   796		uint32_t blendcfg, stride;
   797		uint32_t x, y, src_x, src_y, width, height;
   798		uint32_t roi_w, roi_h;
   799		int lm;
   800	
   801		assert_spin_locked(&mdp5_crtc->cursor.lock);
   802	
   803		lm = mdp5_cstate->pipeline.mixer->lm;
   804	
   805		x = mdp5_crtc->cursor.x;
   806		y = mdp5_crtc->cursor.y;
   807		width = mdp5_crtc->cursor.width;
   808		height = mdp5_crtc->cursor.height;
   809	
   810		stride = width * drm_format_plane_cpp(DRM_FORMAT_ARGB8888, 0);
   811	
   812		get_roi(crtc, &roi_w, &roi_h);
   813	
   814		/* If cusror buffer overlaps due to rotation on the
   815		 * upper or left screen border the pixel offset inside
   816		 * the cursor buffer of the ROI is the positive overlap
   817		 * distance.
   818		 */
   819		if (mdp5_crtc->cursor.x < 0) {
   820			src_x = abs(mdp5_crtc->cursor.x);
   821			x = 0;
   822		} else {
   823			src_x = 0;
   824		}
   825		if (mdp5_crtc->cursor.y < 0) {
   826			src_y = abs(mdp5_crtc->cursor.y);
   827			y = 0;
   828		} else {
   829			src_y = 0;
   830		}
 > 831		DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
   832			x, y, roi_w, roi_h, src_x, src_y);
   833	
   834		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride);
   835		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm),
   836				MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB8888));
   837		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_IMG_SIZE(lm),
   838				MDP5_LM_CURSOR_IMG_SIZE_SRC_H(height) |
   839				MDP5_LM_CURSOR_IMG_SIZE_SRC_W(width));
   840		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(lm),
   841				MDP5_LM_CURSOR_SIZE_ROI_H(roi_h) |
   842				MDP5_LM_CURSOR_SIZE_ROI_W(roi_w));
   843		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm),
   844				MDP5_LM_CURSOR_START_XY_Y_START(y) |
   845				MDP5_LM_CURSOR_START_XY_X_START(x));
   846		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm),
   847				MDP5_LM_CURSOR_XY_SRC_Y(src_y) |
   848				MDP5_LM_CURSOR_XY_SRC_X(src_x));
   849		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm),
   850				mdp5_crtc->cursor.iova);
   851	
   852		blendcfg = MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_EN;
   853		blendcfg |= MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL(cur_alpha);
   854		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BLEND_CONFIG(lm), blendcfg);
   855	}
   856	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38925 bytes --]

[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] drm/msm/display: negative x/y in cursor move
@ 2018-07-16 22:06   ` kbuild test robot
  0 siblings, 0 replies; 11+ messages in thread
From: kbuild test robot @ 2018-07-16 22:06 UTC (permalink / raw)
  To: Carsten Behling
  Cc: kbuild-all, Carsten Behling, Rob Clark, David Airlie,
	Archit Taneja, Sean Paul, Steve Kowalik, Daniel Vetter,
	Viresh Kumar, linux-arm-msm, dri-devel, freedreno, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 5858 bytes --]

Hi Carsten,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robclark/msm-next]
[also build test WARNING on v4.18-rc5 next-20180716]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Carsten-Behling/drm-msm-display-negative-x-y-in-cursor-move/20180717-031351
base:   git://people.freedesktop.org/~robclark/linux msm-next
config: arm64-defconfig (attached as .config)
compiler: aarch64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=arm64 

All warnings (new ones prefixed by >>):

   In file included from include/drm/drm_mm.h:49:0,
                    from include/drm/drmP.h:73,
                    from include/drm/drm_modeset_helper.h:26,
                    from include/drm/drm_crtc_helper.h:44,
                    from drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:22:
   drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c: In function 'mdp5_crtc_restore_cursor':
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:6: warning: format '%s' expects argument of type 'char *', but argument 3 has type 'uint32_t {aka unsigned int}' [-Wformat=]
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
         ^
   include/drm/drm_print.h:285:25: note: in definition of macro 'DRM_DEBUG_DRIVER'
     drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
                            ^~~
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:2: note: in expansion of macro 'DBG'
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
     ^~~
   drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:8: note: format string is defined here
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
          ~^
          %d
   In file included from include/drm/drm_mm.h:49:0,
                    from include/drm/drmP.h:73,
                    from include/drm/drm_modeset_helper.h:26,
                    from include/drm/drm_crtc_helper.h:44,
                    from drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:22:
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:6: warning: format '%d' expects a matching 'int' argument [-Wformat=]
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
         ^
   include/drm/drm_print.h:285:25: note: in definition of macro 'DRM_DEBUG_DRIVER'
     drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
                            ^~~
>> drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:2: note: in expansion of macro 'DBG'
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
     ^~~
   drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c:831:56: note: format string is defined here
     DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
                                                          ~^

vim +831 drivers/gpu/drm/msm/disp/mdp5/mdp5_crtc.c

   789	
   790	static void mdp5_crtc_restore_cursor(struct drm_crtc *crtc)
   791	{
   792		struct mdp5_crtc_state *mdp5_cstate = to_mdp5_crtc_state(crtc->state);
   793		struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
   794		struct mdp5_kms *mdp5_kms = get_kms(crtc);
   795		const enum mdp5_cursor_alpha cur_alpha = CURSOR_ALPHA_PER_PIXEL;
   796		uint32_t blendcfg, stride;
   797		uint32_t x, y, src_x, src_y, width, height;
   798		uint32_t roi_w, roi_h;
   799		int lm;
   800	
   801		assert_spin_locked(&mdp5_crtc->cursor.lock);
   802	
   803		lm = mdp5_cstate->pipeline.mixer->lm;
   804	
   805		x = mdp5_crtc->cursor.x;
   806		y = mdp5_crtc->cursor.y;
   807		width = mdp5_crtc->cursor.width;
   808		height = mdp5_crtc->cursor.height;
   809	
   810		stride = width * drm_format_plane_cpp(DRM_FORMAT_ARGB8888, 0);
   811	
   812		get_roi(crtc, &roi_w, &roi_h);
   813	
   814		/* If cusror buffer overlaps due to rotation on the
   815		 * upper or left screen border the pixel offset inside
   816		 * the cursor buffer of the ROI is the positive overlap
   817		 * distance.
   818		 */
   819		if (mdp5_crtc->cursor.x < 0) {
   820			src_x = abs(mdp5_crtc->cursor.x);
   821			x = 0;
   822		} else {
   823			src_x = 0;
   824		}
   825		if (mdp5_crtc->cursor.y < 0) {
   826			src_y = abs(mdp5_crtc->cursor.y);
   827			y = 0;
   828		} else {
   829			src_y = 0;
   830		}
 > 831		DBG("%s: x=%d, y=%d roi_w=%d roi_h=%d src_x=%d src_y=%d",
   832			x, y, roi_w, roi_h, src_x, src_y);
   833	
   834		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_STRIDE(lm), stride);
   835		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_FORMAT(lm),
   836				MDP5_LM_CURSOR_FORMAT_FORMAT(CURSOR_FMT_ARGB8888));
   837		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_IMG_SIZE(lm),
   838				MDP5_LM_CURSOR_IMG_SIZE_SRC_H(height) |
   839				MDP5_LM_CURSOR_IMG_SIZE_SRC_W(width));
   840		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_SIZE(lm),
   841				MDP5_LM_CURSOR_SIZE_ROI_H(roi_h) |
   842				MDP5_LM_CURSOR_SIZE_ROI_W(roi_w));
   843		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_START_XY(lm),
   844				MDP5_LM_CURSOR_START_XY_Y_START(y) |
   845				MDP5_LM_CURSOR_START_XY_X_START(x));
   846		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_XY(lm),
   847				MDP5_LM_CURSOR_XY_SRC_Y(src_y) |
   848				MDP5_LM_CURSOR_XY_SRC_X(src_x));
   849		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BASE_ADDR(lm),
   850				mdp5_crtc->cursor.iova);
   851	
   852		blendcfg = MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_EN;
   853		blendcfg |= MDP5_LM_CURSOR_BLEND_CONFIG_BLEND_ALPHA_SEL(cur_alpha);
   854		mdp5_write(mdp5_kms, REG_MDP5_LM_CURSOR_BLEND_CONFIG(lm), blendcfg);
   855	}
   856	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 38925 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2] drm/msm/display: negative x/y in cursor move
  2018-07-16 22:06   ` kbuild test robot
@ 2018-07-16 23:03       ` Carsten Behling
  -1 siblings, 0 replies; 11+ messages in thread
From: Carsten Behling @ 2018-07-16 23:03 UTC (permalink / raw)
  Cc: Archit Taneja, Carsten Behling, Steve Kowalik, David Airlie,
	Daniel Vetter, linux-arm-msm-u79uwXL29TY76Z2rM5mHXA,
	Maarten Lankhorst, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark, Sean Paul,
	Viresh Kumar, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

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(-)

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);
 
-- 
2.14.1

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2] drm/msm/display: negative x/y in cursor move
@ 2018-07-16 23:03       ` Carsten Behling
  0 siblings, 0 replies; 11+ messages in thread
From: Carsten Behling @ 2018-07-16 23:03 UTC (permalink / raw)
  Cc: Carsten Behling, Rob Clark, David Airlie, Archit Taneja,
	Sean Paul, Daniel Vetter, Maarten Lankhorst, Steve Kowalik,
	Viresh Kumar, linux-arm-msm, dri-devel, freedreno, linux-kernel

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(-)

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);
 
-- 
2.14.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] drm/msm/display: negative x/y in cursor move
  2018-07-16 23:03       ` Carsten Behling
@ 2018-07-24 17:33           ` Archit Taneja
  -1 siblings, 0 replies; 11+ messages in thread
From: Archit Taneja @ 2018-07-24 17:33 UTC (permalink / raw)
  To: Carsten Behling
  Cc: Carsten Behling, Steve Kowalik, David Airlie, Daniel Vetter,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Maarten Lankhorst,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark, Sean Paul,
	Viresh Kumar, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW

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);
>   
> 
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] drm/msm/display: negative x/y in cursor move
@ 2018-07-24 17:33           ` Archit Taneja
  0 siblings, 0 replies; 11+ messages in thread
From: Archit Taneja @ 2018-07-24 17:33 UTC (permalink / raw)
  To: Carsten Behling
  Cc: Carsten Behling, Rob Clark, David Airlie, Sean Paul,
	Daniel Vetter, Maarten Lankhorst, Steve Kowalik, Viresh Kumar,
	linux-arm-msm, dri-devel, freedreno, linux-kernel

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);
>   
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] drm/msm/display: negative x/y in cursor move
       [not found]           ` <99e72f0c-e753-cff9-9a58-50d919d472a5-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
@ 2018-07-25 15:10             ` Carsten Behling
       [not found]               ` <CAPuGWB9oBN4U6OS8FLDQ1m6eO3n4mT-UG3QWHdjuM9XgSLG8iQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
  0 siblings, 1 reply; 11+ messages in thread
From: Carsten Behling @ 2018-07-25 15:10 UTC (permalink / raw)
  To: Archit Taneja
  Cc: Carsten Behling, Steve Kowalik, David Airlie, Daniel Vetter,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Maarten Lankhorst,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark, Sean Paul,
	Viresh Kumar, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW


[-- Attachment #1.1: Type: text/plain, Size: 7278 bytes --]

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-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> 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);
>>
>>
>

[-- Attachment #1.2: Type: text/html, Size: 10773 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] drm/msm/display: negative x/y in cursor move
  2018-07-25 15:10             ` Carsten Behling
@ 2018-07-26  7:25                   ` Archit Taneja
  0 siblings, 0 replies; 11+ messages in thread
From: Archit Taneja @ 2018-07-26  7:25 UTC (permalink / raw)
  To: Carsten Behling
  Cc: Carsten Behling, Steve Kowalik, David Airlie, Daniel Vetter,
	linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Maarten Lankhorst,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Rob Clark, Sean Paul,
	Viresh Kumar, freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW



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);
> 
> 
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2] drm/msm/display: negative x/y in cursor move
@ 2018-07-26  7:25                   ` Archit Taneja
  0 siblings, 0 replies; 11+ messages in thread
From: Archit Taneja @ 2018-07-26  7:25 UTC (permalink / raw)
  To: Carsten Behling
  Cc: Carsten Behling, Rob Clark, David Airlie, Sean Paul,
	Daniel Vetter, Maarten Lankhorst, Steve Kowalik, Viresh Kumar,
	linux-arm-msm, dri-devel, freedreno, linux-kernel



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);
> 
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-07-26  7:25 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-16 17:49 [PATCH] drm/msm/display: negative x/y in cursor move Carsten Behling
2018-07-16 17:49 ` Carsten Behling
2018-07-16 22:06 ` kbuild test robot
2018-07-16 22:06   ` kbuild test robot
     [not found]   ` <201807170504.KBRX6Bew%fengguang.wu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
2018-07-16 23:03     ` [PATCH v2] " Carsten Behling
2018-07-16 23:03       ` Carsten Behling
     [not found]       ` <20180716230314.3527-1-carsten.behling-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2018-07-24 17:33         ` Archit Taneja
2018-07-24 17:33           ` Archit Taneja
     [not found]           ` <99e72f0c-e753-cff9-9a58-50d919d472a5-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
2018-07-25 15:10             ` Carsten Behling
     [not found]               ` <CAPuGWB9oBN4U6OS8FLDQ1m6eO3n4mT-UG3QWHdjuM9XgSLG8iQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2018-07-26  7:25                 ` Archit Taneja
2018-07-26  7:25                   ` Archit Taneja

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.