[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <1763b39b-9898-bf5a-9afc-32d8e753bbbc@codeaurora.org>
Date: Thu, 26 Jul 2018 12:55:24 +0530
From: Archit Taneja <architt@...eaurora.org>
To: Carsten Behling <carsten.behling@...glemail.com>
Cc: Carsten Behling <carsten.behling@...il.com>,
Rob Clark <robdclark@...il.com>,
David Airlie <airlied@...ux.ie>,
Sean Paul <seanpaul@...omium.org>,
Daniel Vetter <daniel.vetter@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Steve Kowalik <steven@...ontsleep.org>,
Viresh Kumar <viresh.kumar@...aro.org>,
linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/msm/display: negative x/y in cursor move
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@...eaurora.org
> <mailto:architt@...eaurora.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@...il.com
> <mailto:carsten.behling@...il.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);
>
>
Powered by blists - more mailing lists