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);
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-neg... 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
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);
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;@@ -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));@@ -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;@@ -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;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);*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);
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);
dri-devel@lists.freedesktop.org
-
Archit Taneja -
Carsten Behling -
kbuild test robot