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 <[email protected]>
---
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
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
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 <[email protected]>
---
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
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 <[email protected]>
> ---
> 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,
> 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 <[email protected]>:
> 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 <[email protected]>
>> ---
>> 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);
>>
>>
>
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 <[email protected]
> <mailto:[email protected]>>:
>
> 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 <[email protected]
> <mailto:[email protected]>>
> ---
> 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);
>
>