Re: [PATCH V3] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
Couple thoughs. First, we need to also update
drm_output_prepare_cursor_view to check against the size coming from GBM
instead of against the hard-coded 64x64 it's currently checking against.
Without changing that, we are still restricted to 64x64 regardless of the
GBM checking.
Other questions below.
On Mon, Jul 28, 2014 at 2:30 PM, Alvaro Fernando García
alvarofernandogar...@gmail.com wrote:
Init cursor size to 64x64 if drmGetCap() fails.
Use Mesa GBM_BO_USE_CURSOR define (which removes 64x64 restriction)
Signed-off-by: Alvaro Fernando García alvarofernandogar...@gmail.com
---
src/compositor-drm.c | 41 ++---
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/src/compositor-drm.c b/src/compositor-drm.c
index 7d514e4..9c83b1a 100644
--- a/src/compositor-drm.c
+++ b/src/compositor-drm.c
@@ -55,6 +55,18 @@
#define DRM_CAP_TIMESTAMP_MONOTONIC 0x6
#endif
+#ifndef DRM_CAP_CURSOR_WIDTH
+#define DRM_CAP_CURSOR_WIDTH 0x8
+#endif
+
+#ifndef DRM_CAP_CURSOR_HEIGHT
+#define DRM_CAP_CURSOR_HEIGHT 0x9
+#endif
+
+#ifndef GBM_BO_USE_CURSOR
+#define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64
+#endif
Is GBM_BO_USE_CURSOR a valid vallback for GBM_BO_USE_CURSOR_64x64? What
happens if drmGetCap fails but GBM_BO_USE_CURSOR is defined? Is that going
to be ok?
+
static int option_current_mode = 0;
enum output_config {
@@ -108,6 +120,9 @@ struct drm_compositor {
clockid_t clock;
struct udev_input input;
+
+ uint32_t cursor_width;
+ uint32_t cursor_height;
};
struct drm_mode {
@@ -987,7 +1002,7 @@ drm_output_set_cursor(struct drm_output *output)
(struct drm_compositor *) output-base.compositor;
EGLint handle, stride;
struct gbm_bo *bo;
- uint32_t buf[64 * 64];
+ uint32_t buf[c-cursor_width * c-cursor_height];
unsigned char *s;
int i, x, y;
@@ -1010,7 +1025,7 @@ drm_output_set_cursor(struct drm_output *output)
s = wl_shm_buffer_get_data(buffer-shm_buffer);
wl_shm_buffer_begin_access(buffer-shm_buffer);
for (i = 0; i ev-surface-height; i++)
- memcpy(buf + i * 64, s + i * stride,
+ memcpy(buf + i * c-cursor_width, s + i * stride,
ev-surface-width * 4);
wl_shm_buffer_end_access(buffer-shm_buffer);
@@ -1018,8 +1033,8 @@ drm_output_set_cursor(struct drm_output *output)
weston_log(failed update cursor: %m\n);
handle = gbm_bo_get_handle(bo).s32;
- if (drmModeSetCursor(c-drm.fd,
-output-crtc_id, handle, 64, 64)) {
+ if (drmModeSetCursor(c-drm.fd, output-crtc_id, handle,
+ c-cursor_width, c-cursor_height)) {
weston_log(failed to set cursor: %m\n);
c-cursors_are_broken = 1;
}
@@ -1296,6 +1311,18 @@ init_drm(struct drm_compositor *ec, struct
udev_device *device)
else
ec-clock = CLOCK_REALTIME;
+ ret = drmGetCap(fd, DRM_CAP_CURSOR_WIDTH, cap);
+ if (ret == 0)
+ ec-cursor_width = cap;
+ else
+ ec-cursor_width = 64;
+
+ ret = drmGetCap(fd, DRM_CAP_CURSOR_HEIGHT, cap);
+ if (ret == 0)
+ ec-cursor_height = cap;
+ else
+ ec-cursor_height = 64;
+
I think this was asked before, but never answered. Do we have known bounds
on these values? I guess they come from GBM so we can probably trust that
they're reasonable, but what are the guarantees?
return 0;
}
@@ -1554,15 +1581,15 @@ drm_output_init_egl(struct drm_output *output,
struct drm_compositor *ec)
return -1;
}
- flags = GBM_BO_USE_CURSOR_64X64 | GBM_BO_USE_WRITE;
+ flags = GBM_BO_USE_CURSOR | GBM_BO_USE_WRITE;
for (i = 0; i 2; i++) {
if (output-cursor_bo[i])
continue;
output-cursor_bo[i] =
- gbm_bo_create(ec-gbm, 64, 64, GBM_FORMAT_ARGB,
- flags);
+ gbm_bo_create(ec-gbm, ec-cursor_width,
ec-cursor_height,
+ GBM_FORMAT_ARGB, flags);
}
if (output-cursor_bo[0] == NULL || output-cursor_bo[1] == NULL) {
--
2.0.3
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V3] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
On 29.07.2014 16:01, Jason Ekstrand wrote: Couple thoughs. First, we need to also update drm_output_prepare_cursor_view to check against the size coming from GBM instead of against the hard-coded 64x64 it's currently checking against. Without changing that, we are still restricted to 64x64 regardless of the GBM checking. You mean weston will still refuse to use the hardware cursor for images larger than 64x64 without that change? That sounds like something that should indeed be fixed, though it's not really critical compared to the problem fixed by this patch, which is that the hardware cursor appears corrupted beyond usability on hardware which only supports hardware cursors of sizes other than 64x64. On Mon, Jul 28, 2014 at 2:30 PM, Alvaro Fernando García alvarofernandogar...@gmail.com mailto:alvarofernandogar...@gmail.com wrote: diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 7d514e4..9c83b1a 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -55,6 +55,18 @@ #define DRM_CAP_TIMESTAMP_MONOTONIC 0x6 #endif +#ifndef DRM_CAP_CURSOR_WIDTH +#define DRM_CAP_CURSOR_WIDTH 0x8 +#endif + +#ifndef DRM_CAP_CURSOR_HEIGHT +#define DRM_CAP_CURSOR_HEIGHT 0x9 +#endif + +#ifndef GBM_BO_USE_CURSOR +#define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64 +#endif Is GBM_BO_USE_CURSOR a valid vallback for GBM_BO_USE_CURSOR_64x64? It's the other way around, GBM_BO_USE_CURSOR_64x64 is the fallback for GBM_BO_USE_CURSOR not being defined. Note that GBM_BO_USE_CURSOR has the same value as GBM_BO_USE_CURSOR_64x64 even in gbm.h, I just changed the name to clarify that it's no longer restricted to 64x64. What happens if drmGetCap fails but GBM_BO_USE_CURSOR is defined? Is that going to be ok? The two things are not directly related, but yes, that will work, because weston will use 64x64, which works with all versions of GBM. @@ -1296,6 +1311,18 @@ init_drm(struct drm_compositor *ec, struct udev_device *device) else ec-clock = CLOCK_REALTIME; + ret = drmGetCap(fd, DRM_CAP_CURSOR_WIDTH, cap); + if (ret == 0) + ec-cursor_width = cap; + else + ec-cursor_width = 64; + + ret = drmGetCap(fd, DRM_CAP_CURSOR_HEIGHT, cap); + if (ret == 0) + ec-cursor_height = cap; + else + ec-cursor_height = 64; + I think this was asked before, but never answered. Do we have known bounds on these values? I guess they come from GBM so we can probably trust that they're reasonable, but what are the guarantees? They come from DRM, not GBM, and the values returned depend on the individual DRM driver. If the driver doesn't set the cursor width/height explicitly, the DRM core returns 64 for both. -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V3] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
On Tue, Jul 29, 2014 at 12:17 AM, Michel Dänzer mic...@daenzer.net wrote: On 29.07.2014 16:01, Jason Ekstrand wrote: Couple thoughs. First, we need to also update drm_output_prepare_cursor_view to check against the size coming from GBM instead of against the hard-coded 64x64 it's currently checking against. Without changing that, we are still restricted to 64x64 regardless of the GBM checking. You mean weston will still refuse to use the hardware cursor for images larger than 64x64 without that change? That sounds like something that should indeed be fixed, though it's not really critical compared to the problem fixed by this patch, which is that the hardware cursor appears corrupted beyond usability on hardware which only supports hardware cursors of sizes other than 64x64. Yup. That is exactly what it means. It should be a fairly easy fix. If you'd rather I push this and fix in a follow-up patch, that's probably ok, but let's make sure one is coming. --Jason Ekstrand On Mon, Jul 28, 2014 at 2:30 PM, Alvaro Fernando García alvarofernandogar...@gmail.com mailto:alvarofernandogar...@gmail.com wrote: diff --git a/src/compositor-drm.c b/src/compositor-drm.c index 7d514e4..9c83b1a 100644 --- a/src/compositor-drm.c +++ b/src/compositor-drm.c @@ -55,6 +55,18 @@ #define DRM_CAP_TIMESTAMP_MONOTONIC 0x6 #endif +#ifndef DRM_CAP_CURSOR_WIDTH +#define DRM_CAP_CURSOR_WIDTH 0x8 +#endif + +#ifndef DRM_CAP_CURSOR_HEIGHT +#define DRM_CAP_CURSOR_HEIGHT 0x9 +#endif + +#ifndef GBM_BO_USE_CURSOR +#define GBM_BO_USE_CURSOR GBM_BO_USE_CURSOR_64X64 +#endif Is GBM_BO_USE_CURSOR a valid vallback for GBM_BO_USE_CURSOR_64x64? It's the other way around, GBM_BO_USE_CURSOR_64x64 is the fallback for GBM_BO_USE_CURSOR not being defined. Note that GBM_BO_USE_CURSOR has the same value as GBM_BO_USE_CURSOR_64x64 even in gbm.h, I just changed the name to clarify that it's no longer restricted to 64x64. What happens if drmGetCap fails but GBM_BO_USE_CURSOR is defined? Is that going to be ok? The two things are not directly related, but yes, that will work, because weston will use 64x64, which works with all versions of GBM. @@ -1296,6 +1311,18 @@ init_drm(struct drm_compositor *ec, struct udev_device *device) else ec-clock = CLOCK_REALTIME; + ret = drmGetCap(fd, DRM_CAP_CURSOR_WIDTH, cap); + if (ret == 0) + ec-cursor_width = cap; + else + ec-cursor_width = 64; + + ret = drmGetCap(fd, DRM_CAP_CURSOR_HEIGHT, cap); + if (ret == 0) + ec-cursor_height = cap; + else + ec-cursor_height = 64; + I think this was asked before, but never answered. Do we have known bounds on these values? I guess they come from GBM so we can probably trust that they're reasonable, but what are the guarantees? They come from DRM, not GBM, and the values returned depend on the individual DRM driver. If the driver doesn't set the cursor width/height explicitly, the DRM core returns 64 for both. -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V3] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
On 29.07.2014 16:36, Jason Ekstrand wrote: On Tue, Jul 29, 2014 at 12:17 AM, Michel Dänzer mic...@daenzer.net mailto:mic...@daenzer.net wrote: On 29.07.2014 16:01, Jason Ekstrand wrote: Couple thoughs. First, we need to also update drm_output_prepare_cursor_view to check against the size coming from GBM instead of against the hard-coded 64x64 it's currently checking against. Without changing that, we are still restricted to 64x64 regardless of the GBM checking. You mean weston will still refuse to use the hardware cursor for images larger than 64x64 without that change? That sounds like something that should indeed be fixed, though it's not really critical compared to the problem fixed by this patch, which is that the hardware cursor appears corrupted beyond usability on hardware which only supports hardware cursors of sizes other than 64x64. Yup. That is exactly what it means. It should be a fairly easy fix. If you'd rather I push this and fix in a follow-up patch, that's probably ok, That would be great. but let's make sure one is coming. Alvaro, do you want to send a follow-up patch addressing the issue Jason described above? -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V3] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
I pushed this one. Let's get a follow-up that lets weston actually use the bigger cursors. It would also be good to hack together a little client that attaches a big cursor so we can verify that it's working. I don't think we need to put it in the repo, I'd just like proof that we're actually taking advantage of our new-found big cursors. Thanks, --Jason Ekstrand On Tue, Jul 29, 2014 at 12:40 AM, Michel Dänzer mic...@daenzer.net wrote: On 29.07.2014 16:36, Jason Ekstrand wrote: On Tue, Jul 29, 2014 at 12:17 AM, Michel Dänzer mic...@daenzer.net mailto:mic...@daenzer.net wrote: On 29.07.2014 16:01, Jason Ekstrand wrote: Couple thoughs. First, we need to also update drm_output_prepare_cursor_view to check against the size coming from GBM instead of against the hard-coded 64x64 it's currently checking against. Without changing that, we are still restricted to 64x64 regardless of the GBM checking. You mean weston will still refuse to use the hardware cursor for images larger than 64x64 without that change? That sounds like something that should indeed be fixed, though it's not really critical compared to the problem fixed by this patch, which is that the hardware cursor appears corrupted beyond usability on hardware which only supports hardware cursors of sizes other than 64x64. Yup. That is exactly what it means. It should be a fairly easy fix. If you'd rather I push this and fix in a follow-up patch, that's probably ok, That would be great. but let's make sure one is coming. Alvaro, do you want to send a follow-up patch addressing the issue Jason described above? -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V3] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
On Tue, Jul 29, 2014 at 12:55:24AM -0700, Jason Ekstrand wrote: I pushed this one. Let's get a follow-up that lets weston actually use the bigger cursors. It would also be good to hack together a little client that attaches a big cursor so we can verify that it's working. I don't think we need to put it in the repo, I'd just like proof that we're actually taking advantage of our new-found big cursors. Just an aside: With recent kernels intel hw supporst 64x64, 128x128 and 256x256. On all generations (down to gen2). -Daniel Thanks, --Jason Ekstrand On Tue, Jul 29, 2014 at 12:40 AM, Michel Dänzer mic...@daenzer.net wrote: On 29.07.2014 16:36, Jason Ekstrand wrote: On Tue, Jul 29, 2014 at 12:17 AM, Michel Dänzer mic...@daenzer.net mailto:mic...@daenzer.net wrote: On 29.07.2014 16:01, Jason Ekstrand wrote: Couple thoughs. First, we need to also update drm_output_prepare_cursor_view to check against the size coming from GBM instead of against the hard-coded 64x64 it's currently checking against. Without changing that, we are still restricted to 64x64 regardless of the GBM checking. You mean weston will still refuse to use the hardware cursor for images larger than 64x64 without that change? That sounds like something that should indeed be fixed, though it's not really critical compared to the problem fixed by this patch, which is that the hardware cursor appears corrupted beyond usability on hardware which only supports hardware cursors of sizes other than 64x64. Yup. That is exactly what it means. It should be a fairly easy fix. If you'd rather I push this and fix in a follow-up patch, that's probably ok, That would be great. but let's make sure one is coming. Alvaro, do you want to send a follow-up patch addressing the issue Jason described above? -- Earthling Michel Dänzer| http://www.amd.com Libre software enthusiast |Mesa and X developer ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V3] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
On Tue, 29 Jul 2014 13:19:10 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Tue, Jul 29, 2014 at 12:55:24AM -0700, Jason Ekstrand wrote: I pushed this one. Let's get a follow-up that lets weston actually use the bigger cursors. It would also be good to hack together a little client that attaches a big cursor so we can verify that it's working. I don't think we need to put it in the repo, I'd just like proof that we're actually taking advantage of our new-found big cursors. Just an aside: With recent kernels intel hw supporst 64x64, 128x128 and 256x256. On all generations (down to gen2). Ok, so which one of those will we get through drmGetCap()? I think it would not be nice, if we receive the values 256x256, because then Weston will pad *all* cursor images to 256x256, even if 64x64 would suffice. So possibly quite a waste there, first memcpy and then full-sized gbm_bo_write for every cursor change. If drmGetCap returns 64, 128, or 256, how would we infer all the valid sizes? All powers-of-two larger than 64x64, included? Is e.g. 64x256 valid? Thanks, pq ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH V3] Do not assume 64x64 cursor, added support for other sizes (like in AMD Kaveri, 128x128).
On Tue, Jul 29, 2014 at 3:03 PM, Pekka Paalanen ppaala...@gmail.com wrote: On Tue, 29 Jul 2014 13:19:10 +0200 Daniel Vetter dan...@ffwll.ch wrote: On Tue, Jul 29, 2014 at 12:55:24AM -0700, Jason Ekstrand wrote: I pushed this one. Let's get a follow-up that lets weston actually use the bigger cursors. It would also be good to hack together a little client that attaches a big cursor so we can verify that it's working. I don't think we need to put it in the repo, I'd just like proof that we're actually taking advantage of our new-found big cursors. Just an aside: With recent kernels intel hw supporst 64x64, 128x128 and 256x256. On all generations (down to gen2). Ok, so which one of those will we get through drmGetCap()? I think it would not be nice, if we receive the values 256x256, because then Weston will pad *all* cursor images to 256x256, even if 64x64 would suffice. So possibly quite a waste there, first memcpy and then full-sized gbm_bo_write for every cursor change. If drmGetCap returns 64, 128, or 256, how would we infer all the valid sizes? All powers-of-two larger than 64x64, included? Is e.g. 64x256 valid? I added the drmGetCap cursor support for radeon as our hw only supports fixed size hw cursors and there is other hardware out there with similar limitations. I'm not sure what the best way to handle this would be for hw that supports multiple cursor sizes. I think perhaps the recent KMS overlay/plane changes cover this by exposing cursors as planes. Alex ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/wayland-devel
