* [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
* 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).