* [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 0 siblings, 1 reply; 5+ 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] 5+ messages in thread
* Re: [PATCH] drm/msm/display: negative x/y in cursor move 2018-07-16 17:49 [PATCH] drm/msm/display: negative x/y in cursor move Carsten Behling @ 2018-07-16 22:06 ` kbuild test robot 2018-07-16 23:03 ` [PATCH v2] " Carsten Behling 0 siblings, 1 reply; 5+ 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] 5+ 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 2018-07-24 17:33 ` Archit Taneja 0 siblings, 1 reply; 5+ 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] 5+ messages in thread
* Re: [PATCH v2] drm/msm/display: negative x/y in cursor move 2018-07-16 23:03 ` [PATCH v2] " Carsten Behling @ 2018-07-24 17:33 ` Archit Taneja [not found] ` <CAPuGWB9oBN4U6OS8FLDQ1m6eO3n4mT-UG3QWHdjuM9XgSLG8iQ@mail.gmail.com> 0 siblings, 1 reply; 5+ 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] 5+ messages in thread
[parent not found: <CAPuGWB9oBN4U6OS8FLDQ1m6eO3n4mT-UG3QWHdjuM9XgSLG8iQ@mail.gmail.com>]
* Re: [PATCH v2] drm/msm/display: negative x/y in cursor move [not found] ` <CAPuGWB9oBN4U6OS8FLDQ1m6eO3n4mT-UG3QWHdjuM9XgSLG8iQ@mail.gmail.com> @ 2018-07-26 7:25 ` Archit Taneja 0 siblings, 0 replies; 5+ 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] 5+ messages in thread
end of thread, other threads:[~2018-07-26 7:25 UTC | newest]
Thread overview: 5+ 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 22:06 ` kbuild test robot
2018-07-16 23:03 ` [PATCH v2] " Carsten Behling
2018-07-24 17:33 ` Archit Taneja
[not found] ` <CAPuGWB9oBN4U6OS8FLDQ1m6eO3n4mT-UG3QWHdjuM9XgSLG8iQ@mail.gmail.com>
2018-07-26 7:25 ` Archit Taneja
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).