| Message ID | 1425522384-40559-2-git-send-email-hshi@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Mar 04, 2015 at 06:26:24PM -0800, Haixia Shi wrote: > Signed-off-by: Haixia Shi <hshi@chromium.org> > Reviewed-by: Stéphane Marchesin <marcheu@chromium.org> > Tested-by: Haixia Shi <hshi@chromium.org> > --- > +static int udl_cursor_download(struct udl_cursor *cursor, > + struct drm_gem_object *obj) > +{ > + struct udl_gem_object *udl_gem_obj = to_udl_bo(obj); > + uint32_t *src_ptr, *dst_ptr; > + size_t i; > + > + int ret = udl_gem_vmap(udl_gem_obj); > + if (ret != 0) { > + DRM_ERROR("failed to vmap cursor\n"); > + return ret; > + } > + > + src_ptr = udl_gem_obj->vmapping; > + dst_ptr = cursor->buffer; > + for (i = 0; i < UDL_CURSOR_BUF; ++i) > + dst_ptr[i] = cursor_blend_val32(le32_to_cpu(src_ptr[i])); You never verify that udl_gem_obj->size >= UDL_CURSOR_BUF*sizeof(uint32_t) > @@ -220,14 +216,21 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, > return 0; > cmd = urb->transfer_buffer; > > + mutex_lock(&dev->struct_mutex); > + if (udl_cursor_alloc(&cursor_copy) == 0) > + udl_cursor_copy(cursor_copy, udl->cursor); > + mutex_unlock(&dev->struct_mutex); Keeping a reference to the udl->cursor->buffer and making it COW looks quite attrative now. Note that struct_mutex is completely the wrong lock to be using here! It is not the lock you hold when you are modifing the cursor. * cursor = udl_cursor_get(udl->cursor); return NULL on error and make the subsequent code deal with it (rather than discarding the whole frame update). But if you take the COW reference approach it should not fail (the failure will only occur when modifying the cursor, where the error can be propagated back to the user immediately). > +static int udl_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file, > + uint32_t handle, uint32_t width, uint32_t height) > +{ > + struct drm_device *dev = crtc->dev; > + struct udl_device *udl = dev->dev_private; > + int ret; > + > + mutex_lock(&dev->struct_mutex); > + ret = udl_cursor_set(crtc, file, handle, width, height, udl->cursor); > + mutex_unlock(&dev->struct_mutex); * Oh, I see. udl_crtc_cursor_set() is already locked, so you don't need struct_mutex as well. > + if (ret) { > + DRM_ERROR("Failed to set UDL cursor\n"); > + return ret; > + } > + > + return udl_crtc_page_flip(crtc, NULL, NULL, 0); You have to be kidding me! You redraw the entire frame because the cursor changes is is moved! We might as well leave the cursor to userspace, it will be much more performant. -Chris
Hi On Thu, Mar 5, 2015 at 3:26 AM, Haixia Shi <hshi@chromium.org> wrote: > Signed-off-by: Haixia Shi <hshi@chromium.org> > Reviewed-by: Stéphane Marchesin <marcheu@chromium.org> > Tested-by: Haixia Shi <hshi@chromium.org> > --- > drivers/gpu/drm/udl/Makefile | 2 +- > drivers/gpu/drm/udl/udl_cursor.c | 156 +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/udl/udl_cursor.h | 46 +++++++++++ > drivers/gpu/drm/udl/udl_drv.h | 4 + > drivers/gpu/drm/udl/udl_fb.c | 31 ++++---- > drivers/gpu/drm/udl/udl_gem.c | 3 + > drivers/gpu/drm/udl/udl_main.c | 10 +++ > drivers/gpu/drm/udl/udl_modeset.c | 45 +++++++++++ > drivers/gpu/drm/udl/udl_transfer.c | 36 ++++++++- > 9 files changed, 317 insertions(+), 16 deletions(-) > create mode 100644 drivers/gpu/drm/udl/udl_cursor.c > create mode 100644 drivers/gpu/drm/udl/udl_cursor.h Why pretend the hw supports cursors if it clearly does not? I don't see why we cannot leave this to user-space (which can apply optimizations for software-only devices, which the kernel cannot). And a commit-message would also be nice. Thanks David > diff --git a/drivers/gpu/drm/udl/Makefile b/drivers/gpu/drm/udl/Makefile > index 195bcac..67ccab4 100644 > --- a/drivers/gpu/drm/udl/Makefile > +++ b/drivers/gpu/drm/udl/Makefile > @@ -1,6 +1,6 @@ > > ccflags-y := -Iinclude/drm > > -udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o udl_fb.o udl_transfer.o udl_gem.o udl_dmabuf.o > +udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o udl_fb.o udl_transfer.o udl_gem.o udl_dmabuf.o udl_cursor.o > > obj-$(CONFIG_DRM_UDL) := udl.o > diff --git a/drivers/gpu/drm/udl/udl_cursor.c b/drivers/gpu/drm/udl/udl_cursor.c > new file mode 100644 > index 0000000..3b38ef5 > --- /dev/null > +++ b/drivers/gpu/drm/udl/udl_cursor.c > @@ -0,0 +1,156 @@ > +/* > + * udl_cursor.c > + * > + * Copyright (c) 2015 The Chromium OS Authors > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <drm/drmP.h> > +#include <drm/drm_crtc_helper.h> > + > +#include "udl_cursor.h" > +#include "udl_drv.h" > + > +#define UDL_CURSOR_W 64 > +#define UDL_CURSOR_H 64 > +#define UDL_CURSOR_BUF (UDL_CURSOR_W * UDL_CURSOR_H) > + > +/* > + * UDL drm cursor private structure. > + */ > +struct udl_cursor { > + uint32_t buffer[UDL_CURSOR_BUF]; > + bool enabled; > + int x; > + int y; > +}; > + > +int udl_cursor_alloc(struct udl_cursor **cursor) > +{ > + struct udl_cursor *new_cursor = kzalloc(sizeof(struct udl_cursor), > + GFP_KERNEL); > + if (!new_cursor) > + return -ENOMEM; > + *cursor = new_cursor; > + return 0; > +} > + > +void udl_cursor_free(struct udl_cursor *cursor) > +{ > + BUG_ON(!cursor); > + kfree(cursor); > +} > + > +void udl_cursor_copy(struct udl_cursor *dst, struct udl_cursor *src) > +{ > + memcpy(dst, src, sizeof(struct udl_cursor)); > +} > + > +bool udl_cursor_enabled(struct udl_cursor *cursor) > +{ > + return cursor->enabled; > +} > + > +void udl_cursor_get_hline(struct udl_cursor *cursor, int x, int y, > + struct udl_cursor_hline *hline) > +{ > + if (!cursor || !cursor->enabled || > + x >= cursor->x + UDL_CURSOR_W || > + y < cursor->y || y >= cursor->y + UDL_CURSOR_H) { > + hline->buffer = NULL; > + return; > + } > + > + hline->buffer = &cursor->buffer[UDL_CURSOR_W * (y - cursor->y)]; > + hline->width = UDL_CURSOR_W; > + hline->offset = x - cursor->x; > +} > + > +/* > + * Return pre-computed cursor blend value defined as: > + * R: 5 bits (bit 0:4) > + * G: 6 bits (bit 5:10) > + * B: 5 bits (bit 11:15) > + * A: 7 bits (bit 16:22) > + */ > +static uint32_t cursor_blend_val32(uint32_t pix) > +{ > + /* range of alpha_scaled is 0..64 */ > + uint32_t alpha_scaled = ((pix >> 24) * 65) >> 8; > + return ((pix >> 3) & 0x1f) | > + ((pix >> 5) & 0x7e0) | > + ((pix >> 8) & 0xf800) | > + (alpha_scaled << 16); > +} > + > +static int udl_cursor_download(struct udl_cursor *cursor, > + struct drm_gem_object *obj) > +{ > + struct udl_gem_object *udl_gem_obj = to_udl_bo(obj); > + uint32_t *src_ptr, *dst_ptr; > + size_t i; > + > + int ret = udl_gem_vmap(udl_gem_obj); > + if (ret != 0) { > + DRM_ERROR("failed to vmap cursor\n"); > + return ret; > + } > + > + src_ptr = udl_gem_obj->vmapping; > + dst_ptr = cursor->buffer; > + for (i = 0; i < UDL_CURSOR_BUF; ++i) > + dst_ptr[i] = cursor_blend_val32(le32_to_cpu(src_ptr[i])); > + return 0; > +} > + > +int udl_cursor_set(struct drm_crtc *crtc, struct drm_file *file, > + uint32_t handle, uint32_t width, uint32_t height, > + struct udl_cursor *cursor) > +{ > + if (handle) { > + struct drm_gem_object *obj; > + int err; > + /* Currently we only support 64x64 cursors */ > + if (width != UDL_CURSOR_W || height != UDL_CURSOR_H) { > + DRM_ERROR("we currently only support %dx%d cursors\n", > + UDL_CURSOR_W, UDL_CURSOR_H); > + return -EINVAL; > + } > + obj = drm_gem_object_lookup(crtc->dev, file, handle); > + if (!obj) { > + DRM_ERROR("failed to lookup gem object.\n"); > + return -EINVAL; > + } > + err = udl_cursor_download(cursor, obj); > + drm_gem_object_unreference(obj); > + if (err != 0) { > + DRM_ERROR("failed to copy cursor.\n"); > + return err; > + } > + cursor->enabled = true; > + } else { > + cursor->enabled = false; > + } > + > + return 0; > +} > + > +int udl_cursor_move(struct drm_crtc *crtc, int x, int y, > + struct udl_cursor *cursor) > +{ > + cursor->x = x; > + cursor->y = y; > + return 0; > +} > diff --git a/drivers/gpu/drm/udl/udl_cursor.h b/drivers/gpu/drm/udl/udl_cursor.h > new file mode 100644 > index 0000000..4b83f5b > --- /dev/null > +++ b/drivers/gpu/drm/udl/udl_cursor.h > @@ -0,0 +1,46 @@ > +/* > + * udl_cursor.h > + * > + * Copyright (c) 2015 The Chromium OS Authors > + * > + * This program is free software; you can redistribute it and/or modify it > + * under the terms of the GNU General Public License as published by the > + * Free Software Foundation; either version 2 of the License, or (at your > + * option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#ifndef _UDL_CURSOR_H_ > +#define _UDL_CURSOR_H_ > + > +#include <linux/module.h> > +#include <drm/drmP.h> > +#include <drm/drm_crtc.h> > + > +struct udl_cursor; > +struct udl_cursor_hline { > + uint32_t *buffer; > + int width; > + int offset; > +}; > + > +extern int udl_cursor_alloc(struct udl_cursor **cursor); > +extern void udl_cursor_free(struct udl_cursor *cursor); > +extern void udl_cursor_copy(struct udl_cursor *dst, struct udl_cursor *src); > +extern bool udl_cursor_enabled(struct udl_cursor *cursor); > +extern void udl_cursor_get_hline(struct udl_cursor *cursor, int x, int y, > + struct udl_cursor_hline *hline); > +extern int udl_cursor_set(struct drm_crtc *crtc, struct drm_file *file, > + uint32_t handle, uint32_t width, uint32_t height, > + struct udl_cursor *cursor); > +extern int udl_cursor_move(struct drm_crtc *crtc, int x, int y, > + struct udl_cursor *cursor); > + > +#endif > diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h > index b1e9fae..9fe4520 100644 > --- a/drivers/gpu/drm/udl/udl_drv.h > +++ b/drivers/gpu/drm/udl/udl_drv.h > @@ -47,6 +47,8 @@ struct urb_list { > }; > > struct udl_fbdev; > +struct udl_cursor; > +struct udl_cursor_hline; > struct udl_flip_queue; > > struct udl_device { > @@ -60,6 +62,7 @@ struct udl_device { > atomic_t lost_pixels; /* 1 = a render op failed. Need screen refresh */ > > struct udl_fbdev *fbdev; > + struct udl_cursor *cursor; > char mode_buf[1024]; > uint32_t mode_buf_len; > atomic_t bytes_rendered; /* raw pixel-bytes driver asked to render */ > @@ -116,6 +119,7 @@ udl_fb_user_fb_create(struct drm_device *dev, > int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr, > const char *front, char **urb_buf_ptr, > u32 byte_offset, u32 device_byte_offset, u32 byte_width, > + struct udl_cursor_hline *cursor_hline, > int *ident_ptr, int *sent_ptr); > > int udl_dumb_create(struct drm_file *file_priv, > diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c > index 5fc16ce..a4ccbc1 100644 > --- a/drivers/gpu/drm/udl/udl_fb.c > +++ b/drivers/gpu/drm/udl/udl_fb.c > @@ -19,6 +19,7 @@ > #include <drm/drm_crtc.h> > #include <drm/drm_crtc_helper.h> > #include "udl_drv.h" > +#include "udl_cursor.h" > > #include <drm/drm_fb_helper.h> > > @@ -116,8 +117,8 @@ static void udlfb_dpy_deferred_io(struct fb_info *info, > if (udl_render_hline(dev, (ufbdev->ufb.base.bits_per_pixel / 8), > &urb, (char *) info->fix.smem_start, > &cmd, cur->index << PAGE_SHIFT, > - cur->index << PAGE_SHIFT, > - PAGE_SIZE, &bytes_identical, &bytes_sent)) > + cur->index << PAGE_SHIFT, PAGE_SIZE, NULL, > + &bytes_identical, &bytes_sent)) > goto error; > bytes_rendered += PAGE_SIZE; > } > @@ -156,20 +157,15 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, > int x2, y2; > bool store_for_later = false; > unsigned long flags; > + struct udl_cursor *cursor_copy = NULL; > > if (!fb->active_16) > return 0; > > - if (!fb->obj->vmapping) { > - ret = udl_gem_vmap(fb->obj); > - if (ret == -ENOMEM) { > - DRM_ERROR("failed to vmap fb\n"); > - return 0; > - } > - if (!fb->obj->vmapping) { > - DRM_ERROR("failed to vmapping\n"); > - return 0; > - } > + ret = udl_gem_vmap(fb->obj); > + if (ret) { > + DRM_ERROR("failed to vmap fb\n"); > + return 0; > } > > aligned_x = DL_ALIGN_DOWN(x, sizeof(unsigned long)); > @@ -220,14 +216,21 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, > return 0; > cmd = urb->transfer_buffer; > > + mutex_lock(&dev->struct_mutex); > + if (udl_cursor_alloc(&cursor_copy) == 0) > + udl_cursor_copy(cursor_copy, udl->cursor); > + mutex_unlock(&dev->struct_mutex); > + > for (i = y; i <= y2 ; i++) { > const int line_offset = fb->base.pitches[0] * i; > const int byte_offset = line_offset + (x * bpp); > const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp); > + struct udl_cursor_hline cursor_hline; > + udl_cursor_get_hline(cursor_copy, x, i, &cursor_hline); > if (udl_render_hline(dev, bpp, &urb, > (char *) fb->obj->vmapping, > &cmd, byte_offset, dev_byte_offset, > - (x2 - x + 1) * bpp, > + (x2 - x + 1) * bpp, &cursor_hline, > &bytes_identical, &bytes_sent)) > goto error; > } > @@ -241,6 +244,8 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, > udl_urb_completion(urb); > > error: > + if (cursor_copy) > + udl_cursor_free(cursor_copy); > atomic_add(bytes_sent, &udl->bytes_sent); > atomic_add(bytes_identical, &udl->bytes_identical); > atomic_add(width*height*bpp, &udl->bytes_rendered); > diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c > index 2a0a784..9d950ae 100644 > --- a/drivers/gpu/drm/udl/udl_gem.c > +++ b/drivers/gpu/drm/udl/udl_gem.c > @@ -160,6 +160,9 @@ int udl_gem_vmap(struct udl_gem_object *obj) > int page_count = obj->base.size / PAGE_SIZE; > int ret; > > + if (obj->vmapping) > + return 0; > + > if (obj->base.import_attach) { > obj->vmapping = dma_buf_vmap(obj->base.import_attach->dmabuf); > if (!obj->vmapping) > diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c > index 33dbfb2..1f86779 100644 > --- a/drivers/gpu/drm/udl/udl_main.c > +++ b/drivers/gpu/drm/udl/udl_main.c > @@ -12,6 +12,7 @@ > */ > #include <drm/drmP.h> > #include "udl_drv.h" > +#include "udl_cursor.h" > > /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? */ > #define BULK_SIZE 512 > @@ -307,6 +308,10 @@ int udl_driver_load(struct drm_device *dev, unsigned long flags) > } > > DRM_DEBUG("\n"); > + ret = udl_cursor_alloc(&udl->cursor); > + if (ret) > + goto err; > + > ret = udl_modeset_init(dev); > if (ret) > goto err; > @@ -325,6 +330,8 @@ err_fb: > err: > if (udl->urbs.count) > udl_free_urb_list(dev); > + if (udl->cursor) > + udl_cursor_free(udl->cursor); > kfree(udl); > DRM_ERROR("%d\n", ret); > return ret; > @@ -345,6 +352,9 @@ int udl_driver_unload(struct drm_device *dev) > if (udl->urbs.count) > udl_free_urb_list(dev); > > + if (udl->cursor) > + udl_cursor_free(udl->cursor); > + > udl_fbdev_cleanup(dev); > udl_modeset_cleanup(dev); > kfree(udl); > diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c > index f3804b4..d464286 100644 > --- a/drivers/gpu/drm/udl/udl_modeset.c > +++ b/drivers/gpu/drm/udl/udl_modeset.c > @@ -16,6 +16,7 @@ > #include <drm/drm_crtc_helper.h> > #include <drm/drm_plane_helper.h> > #include "udl_drv.h" > +#include "udl_cursor.h" > > /* > * All DisplayLink bulk operations start with 0xAF, followed by specific code > @@ -471,6 +472,48 @@ static void udl_crtc_commit(struct drm_crtc *crtc) > udl_crtc_dpms(crtc, DRM_MODE_DPMS_ON); > } > > +static int udl_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file, > + uint32_t handle, uint32_t width, uint32_t height) > +{ > + struct drm_device *dev = crtc->dev; > + struct udl_device *udl = dev->dev_private; > + int ret; > + > + mutex_lock(&dev->struct_mutex); > + ret = udl_cursor_set(crtc, file, handle, width, height, udl->cursor); > + mutex_unlock(&dev->struct_mutex); > + > + if (ret) { > + DRM_ERROR("Failed to set UDL cursor\n"); > + return ret; > + } > + > + return udl_crtc_page_flip(crtc, NULL, NULL, 0); > +} > + > +static int udl_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) > +{ > + struct drm_device *dev = crtc->dev; > + struct udl_device *udl = dev->dev_private; > + int ret = 0; > + > + mutex_lock(&dev->struct_mutex); > + if (!udl_cursor_enabled(udl->cursor)) > + goto error; > + ret = udl_cursor_move(crtc, x, y, udl->cursor); > + if (ret) { > + DRM_ERROR("Failed to move UDL cursor\n"); > + goto error; > + } > + mutex_unlock(&dev->struct_mutex); > + > + return udl_crtc_page_flip(crtc, NULL, NULL, 0); > + > +error: > + mutex_unlock(&dev->struct_mutex); > + return ret; > +} > + > static struct drm_crtc_helper_funcs udl_helper_funcs = { > .dpms = udl_crtc_dpms, > .mode_fixup = udl_crtc_mode_fixup, > @@ -484,6 +527,8 @@ static const struct drm_crtc_funcs udl_crtc_funcs = { > .set_config = drm_crtc_helper_set_config, > .destroy = udl_crtc_destroy, > .page_flip = udl_crtc_page_flip, > + .cursor_set = udl_crtc_cursor_set, > + .cursor_move = udl_crtc_cursor_move, > }; > > static int udl_crtc_init(struct drm_device *dev) > diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c > index 917dcb9..2f59603 100644 > --- a/drivers/gpu/drm/udl/udl_transfer.c > +++ b/drivers/gpu/drm/udl/udl_transfer.c > @@ -17,6 +17,7 @@ > > #include <drm/drmP.h> > #include "udl_drv.h" > +#include "udl_cursor.h" > > #define MAX_CMD_PIXELS 255 > > @@ -92,6 +93,19 @@ static inline u16 get_pixel_val16(const uint8_t *pixel, int bpp) > return pixel_val16; > } > > +static inline u16 blend_alpha(const uint16_t pixel_val16, uint32_t blend_val32) > +{ > + uint32_t alpha = (blend_val32 >> 16); > + uint32_t alpha_inv = 64 - alpha; > + > + return (((pixel_val16 & 0x1f) * alpha_inv + > + (blend_val32 & 0x1f) * alpha) >> 6) | > + ((((pixel_val16 & 0x7e0) * alpha_inv + > + (blend_val32 & 0x7e0) * alpha) >> 6) & 0x7e0) | > + ((((pixel_val16 & 0xf800) * alpha_inv + > + (blend_val32 & 0xf800) * alpha) >> 6) & 0xf800); > +} > + > /* > * Render a command stream for an encoded horizontal line segment of pixels. > * > @@ -123,12 +137,16 @@ static void udl_compress_hline16( > const u8 **pixel_start_ptr, > const u8 *const pixel_end, > uint32_t *device_address_ptr, > + struct udl_cursor_hline *cursor_hline, > uint8_t **command_buffer_ptr, > const uint8_t *const cmd_buffer_end, int bpp) > { > const u8 *pixel = *pixel_start_ptr; > uint32_t dev_addr = *device_address_ptr; > uint8_t *cmd = *command_buffer_ptr; > + const uint32_t *cursor_buf = cursor_hline ? cursor_hline->buffer : NULL; > + int cursor_pos = cursor_buf ? cursor_hline->offset : 0; > + int cursor_width = cursor_buf ? cursor_hline->width : 0; > > while ((pixel_end > pixel) && > (cmd_buffer_end - MIN_RLX_CMD_BYTES > cmd)) { > @@ -158,6 +176,11 @@ static void udl_compress_hline16( > > prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp); > pixel_val16 = get_pixel_val16(pixel, bpp); > + if (cursor_buf && cursor_pos >= 0 && > + cursor_pos < cursor_width) { > + pixel_val16 = blend_alpha(pixel_val16, > + cursor_buf[cursor_pos]); > + } > > while (pixel < cmd_pixel_end) { > const u8 *const start = pixel; > @@ -167,12 +190,19 @@ static void udl_compress_hline16( > > cmd += 2; > pixel += bpp; > + cursor_pos++; > > while (pixel < cmd_pixel_end) { > pixel_val16 = get_pixel_val16(pixel, bpp); > + if (cursor_buf && cursor_pos >= 0 && > + cursor_pos < cursor_width) { > + pixel_val16 = blend_alpha(pixel_val16, > + cursor_buf[cursor_pos]); > + } > if (pixel_val16 != repeating_pixel_val16) > break; > pixel += bpp; > + cursor_pos++; > } > > if (unlikely(pixel > start + bpp)) { > @@ -208,6 +238,8 @@ static void udl_compress_hline16( > *command_buffer_ptr = cmd; > *pixel_start_ptr = pixel; > *device_address_ptr = dev_addr; > + if (cursor_buf) > + cursor_hline->offset = cursor_pos; > > return; > } > @@ -221,7 +253,7 @@ static void udl_compress_hline16( > int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr, > const char *front, char **urb_buf_ptr, > u32 byte_offset, u32 device_byte_offset, > - u32 byte_width, > + u32 byte_width, struct udl_cursor_hline *cursor_hline, > int *ident_ptr, int *sent_ptr) > { > const u8 *line_start, *line_end, *next_pixel; > @@ -239,7 +271,7 @@ int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr, > while (next_pixel < line_end) { > > udl_compress_hline16(&next_pixel, > - line_end, &base16, > + line_end, &base16, cursor_hline, > (u8 **) &cmd, (u8 *) cmd_end, bpp); > > if (cmd >= cmd_end) { > -- > 2.2.0.rc0.207.ga3a616c > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Thu, Mar 5, 2015 at 3:54 AM, David Herrmann <dh.herrmann@gmail.com> wrote: > Hi > > Why pretend the hw supports cursors if it clearly does not? I don't > see why we cannot leave this to user-space (which can apply > optimizations for software-only devices, which the kernel cannot). And > a commit-message would also be nice. IMHO implementing the drm cursor functions allows user-space to be platform agnostic. Will address the other comments and send updated patch in this thread, thanks. > > Thanks > David
diff --git a/drivers/gpu/drm/udl/Makefile b/drivers/gpu/drm/udl/Makefile index 195bcac..67ccab4 100644 --- a/drivers/gpu/drm/udl/Makefile +++ b/drivers/gpu/drm/udl/Makefile @@ -1,6 +1,6 @@ ccflags-y := -Iinclude/drm -udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o udl_fb.o udl_transfer.o udl_gem.o udl_dmabuf.o +udl-y := udl_drv.o udl_modeset.o udl_connector.o udl_encoder.o udl_main.o udl_fb.o udl_transfer.o udl_gem.o udl_dmabuf.o udl_cursor.o obj-$(CONFIG_DRM_UDL) := udl.o diff --git a/drivers/gpu/drm/udl/udl_cursor.c b/drivers/gpu/drm/udl/udl_cursor.c new file mode 100644 index 0000000..3b38ef5 --- /dev/null +++ b/drivers/gpu/drm/udl/udl_cursor.c @@ -0,0 +1,156 @@ +/* + * udl_cursor.c + * + * Copyright (c) 2015 The Chromium OS Authors + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <drm/drmP.h> +#include <drm/drm_crtc_helper.h> + +#include "udl_cursor.h" +#include "udl_drv.h" + +#define UDL_CURSOR_W 64 +#define UDL_CURSOR_H 64 +#define UDL_CURSOR_BUF (UDL_CURSOR_W * UDL_CURSOR_H) + +/* + * UDL drm cursor private structure. + */ +struct udl_cursor { + uint32_t buffer[UDL_CURSOR_BUF]; + bool enabled; + int x; + int y; +}; + +int udl_cursor_alloc(struct udl_cursor **cursor) +{ + struct udl_cursor *new_cursor = kzalloc(sizeof(struct udl_cursor), + GFP_KERNEL); + if (!new_cursor) + return -ENOMEM; + *cursor = new_cursor; + return 0; +} + +void udl_cursor_free(struct udl_cursor *cursor) +{ + BUG_ON(!cursor); + kfree(cursor); +} + +void udl_cursor_copy(struct udl_cursor *dst, struct udl_cursor *src) +{ + memcpy(dst, src, sizeof(struct udl_cursor)); +} + +bool udl_cursor_enabled(struct udl_cursor *cursor) +{ + return cursor->enabled; +} + +void udl_cursor_get_hline(struct udl_cursor *cursor, int x, int y, + struct udl_cursor_hline *hline) +{ + if (!cursor || !cursor->enabled || + x >= cursor->x + UDL_CURSOR_W || + y < cursor->y || y >= cursor->y + UDL_CURSOR_H) { + hline->buffer = NULL; + return; + } + + hline->buffer = &cursor->buffer[UDL_CURSOR_W * (y - cursor->y)]; + hline->width = UDL_CURSOR_W; + hline->offset = x - cursor->x; +} + +/* + * Return pre-computed cursor blend value defined as: + * R: 5 bits (bit 0:4) + * G: 6 bits (bit 5:10) + * B: 5 bits (bit 11:15) + * A: 7 bits (bit 16:22) + */ +static uint32_t cursor_blend_val32(uint32_t pix) +{ + /* range of alpha_scaled is 0..64 */ + uint32_t alpha_scaled = ((pix >> 24) * 65) >> 8; + return ((pix >> 3) & 0x1f) | + ((pix >> 5) & 0x7e0) | + ((pix >> 8) & 0xf800) | + (alpha_scaled << 16); +} + +static int udl_cursor_download(struct udl_cursor *cursor, + struct drm_gem_object *obj) +{ + struct udl_gem_object *udl_gem_obj = to_udl_bo(obj); + uint32_t *src_ptr, *dst_ptr; + size_t i; + + int ret = udl_gem_vmap(udl_gem_obj); + if (ret != 0) { + DRM_ERROR("failed to vmap cursor\n"); + return ret; + } + + src_ptr = udl_gem_obj->vmapping; + dst_ptr = cursor->buffer; + for (i = 0; i < UDL_CURSOR_BUF; ++i) + dst_ptr[i] = cursor_blend_val32(le32_to_cpu(src_ptr[i])); + return 0; +} + +int udl_cursor_set(struct drm_crtc *crtc, struct drm_file *file, + uint32_t handle, uint32_t width, uint32_t height, + struct udl_cursor *cursor) +{ + if (handle) { + struct drm_gem_object *obj; + int err; + /* Currently we only support 64x64 cursors */ + if (width != UDL_CURSOR_W || height != UDL_CURSOR_H) { + DRM_ERROR("we currently only support %dx%d cursors\n", + UDL_CURSOR_W, UDL_CURSOR_H); + return -EINVAL; + } + obj = drm_gem_object_lookup(crtc->dev, file, handle); + if (!obj) { + DRM_ERROR("failed to lookup gem object.\n"); + return -EINVAL; + } + err = udl_cursor_download(cursor, obj); + drm_gem_object_unreference(obj); + if (err != 0) { + DRM_ERROR("failed to copy cursor.\n"); + return err; + } + cursor->enabled = true; + } else { + cursor->enabled = false; + } + + return 0; +} + +int udl_cursor_move(struct drm_crtc *crtc, int x, int y, + struct udl_cursor *cursor) +{ + cursor->x = x; + cursor->y = y; + return 0; +} diff --git a/drivers/gpu/drm/udl/udl_cursor.h b/drivers/gpu/drm/udl/udl_cursor.h new file mode 100644 index 0000000..4b83f5b --- /dev/null +++ b/drivers/gpu/drm/udl/udl_cursor.h @@ -0,0 +1,46 @@ +/* + * udl_cursor.h + * + * Copyright (c) 2015 The Chromium OS Authors + * + * This program is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License as published by the + * Free Software Foundation; either version 2 of the License, or (at your + * option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#ifndef _UDL_CURSOR_H_ +#define _UDL_CURSOR_H_ + +#include <linux/module.h> +#include <drm/drmP.h> +#include <drm/drm_crtc.h> + +struct udl_cursor; +struct udl_cursor_hline { + uint32_t *buffer; + int width; + int offset; +}; + +extern int udl_cursor_alloc(struct udl_cursor **cursor); +extern void udl_cursor_free(struct udl_cursor *cursor); +extern void udl_cursor_copy(struct udl_cursor *dst, struct udl_cursor *src); +extern bool udl_cursor_enabled(struct udl_cursor *cursor); +extern void udl_cursor_get_hline(struct udl_cursor *cursor, int x, int y, + struct udl_cursor_hline *hline); +extern int udl_cursor_set(struct drm_crtc *crtc, struct drm_file *file, + uint32_t handle, uint32_t width, uint32_t height, + struct udl_cursor *cursor); +extern int udl_cursor_move(struct drm_crtc *crtc, int x, int y, + struct udl_cursor *cursor); + +#endif diff --git a/drivers/gpu/drm/udl/udl_drv.h b/drivers/gpu/drm/udl/udl_drv.h index b1e9fae..9fe4520 100644 --- a/drivers/gpu/drm/udl/udl_drv.h +++ b/drivers/gpu/drm/udl/udl_drv.h @@ -47,6 +47,8 @@ struct urb_list { }; struct udl_fbdev; +struct udl_cursor; +struct udl_cursor_hline; struct udl_flip_queue; struct udl_device { @@ -60,6 +62,7 @@ struct udl_device { atomic_t lost_pixels; /* 1 = a render op failed. Need screen refresh */ struct udl_fbdev *fbdev; + struct udl_cursor *cursor; char mode_buf[1024]; uint32_t mode_buf_len; atomic_t bytes_rendered; /* raw pixel-bytes driver asked to render */ @@ -116,6 +119,7 @@ udl_fb_user_fb_create(struct drm_device *dev, int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr, const char *front, char **urb_buf_ptr, u32 byte_offset, u32 device_byte_offset, u32 byte_width, + struct udl_cursor_hline *cursor_hline, int *ident_ptr, int *sent_ptr); int udl_dumb_create(struct drm_file *file_priv, diff --git a/drivers/gpu/drm/udl/udl_fb.c b/drivers/gpu/drm/udl/udl_fb.c index 5fc16ce..a4ccbc1 100644 --- a/drivers/gpu/drm/udl/udl_fb.c +++ b/drivers/gpu/drm/udl/udl_fb.c @@ -19,6 +19,7 @@ #include <drm/drm_crtc.h> #include <drm/drm_crtc_helper.h> #include "udl_drv.h" +#include "udl_cursor.h" #include <drm/drm_fb_helper.h> @@ -116,8 +117,8 @@ static void udlfb_dpy_deferred_io(struct fb_info *info, if (udl_render_hline(dev, (ufbdev->ufb.base.bits_per_pixel / 8), &urb, (char *) info->fix.smem_start, &cmd, cur->index << PAGE_SHIFT, - cur->index << PAGE_SHIFT, - PAGE_SIZE, &bytes_identical, &bytes_sent)) + cur->index << PAGE_SHIFT, PAGE_SIZE, NULL, + &bytes_identical, &bytes_sent)) goto error; bytes_rendered += PAGE_SIZE; } @@ -156,20 +157,15 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, int x2, y2; bool store_for_later = false; unsigned long flags; + struct udl_cursor *cursor_copy = NULL; if (!fb->active_16) return 0; - if (!fb->obj->vmapping) { - ret = udl_gem_vmap(fb->obj); - if (ret == -ENOMEM) { - DRM_ERROR("failed to vmap fb\n"); - return 0; - } - if (!fb->obj->vmapping) { - DRM_ERROR("failed to vmapping\n"); - return 0; - } + ret = udl_gem_vmap(fb->obj); + if (ret) { + DRM_ERROR("failed to vmap fb\n"); + return 0; } aligned_x = DL_ALIGN_DOWN(x, sizeof(unsigned long)); @@ -220,14 +216,21 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, return 0; cmd = urb->transfer_buffer; + mutex_lock(&dev->struct_mutex); + if (udl_cursor_alloc(&cursor_copy) == 0) + udl_cursor_copy(cursor_copy, udl->cursor); + mutex_unlock(&dev->struct_mutex); + for (i = y; i <= y2 ; i++) { const int line_offset = fb->base.pitches[0] * i; const int byte_offset = line_offset + (x * bpp); const int dev_byte_offset = (fb->base.width * bpp * i) + (x * bpp); + struct udl_cursor_hline cursor_hline; + udl_cursor_get_hline(cursor_copy, x, i, &cursor_hline); if (udl_render_hline(dev, bpp, &urb, (char *) fb->obj->vmapping, &cmd, byte_offset, dev_byte_offset, - (x2 - x + 1) * bpp, + (x2 - x + 1) * bpp, &cursor_hline, &bytes_identical, &bytes_sent)) goto error; } @@ -241,6 +244,8 @@ int udl_handle_damage(struct udl_framebuffer *fb, int x, int y, udl_urb_completion(urb); error: + if (cursor_copy) + udl_cursor_free(cursor_copy); atomic_add(bytes_sent, &udl->bytes_sent); atomic_add(bytes_identical, &udl->bytes_identical); atomic_add(width*height*bpp, &udl->bytes_rendered); diff --git a/drivers/gpu/drm/udl/udl_gem.c b/drivers/gpu/drm/udl/udl_gem.c index 2a0a784..9d950ae 100644 --- a/drivers/gpu/drm/udl/udl_gem.c +++ b/drivers/gpu/drm/udl/udl_gem.c @@ -160,6 +160,9 @@ int udl_gem_vmap(struct udl_gem_object *obj) int page_count = obj->base.size / PAGE_SIZE; int ret; + if (obj->vmapping) + return 0; + if (obj->base.import_attach) { obj->vmapping = dma_buf_vmap(obj->base.import_attach->dmabuf); if (!obj->vmapping) diff --git a/drivers/gpu/drm/udl/udl_main.c b/drivers/gpu/drm/udl/udl_main.c index 33dbfb2..1f86779 100644 --- a/drivers/gpu/drm/udl/udl_main.c +++ b/drivers/gpu/drm/udl/udl_main.c @@ -12,6 +12,7 @@ */ #include <drm/drmP.h> #include "udl_drv.h" +#include "udl_cursor.h" /* -BULK_SIZE as per usb-skeleton. Can we get full page and avoid overhead? */ #define BULK_SIZE 512 @@ -307,6 +308,10 @@ int udl_driver_load(struct drm_device *dev, unsigned long flags) } DRM_DEBUG("\n"); + ret = udl_cursor_alloc(&udl->cursor); + if (ret) + goto err; + ret = udl_modeset_init(dev); if (ret) goto err; @@ -325,6 +330,8 @@ err_fb: err: if (udl->urbs.count) udl_free_urb_list(dev); + if (udl->cursor) + udl_cursor_free(udl->cursor); kfree(udl); DRM_ERROR("%d\n", ret); return ret; @@ -345,6 +352,9 @@ int udl_driver_unload(struct drm_device *dev) if (udl->urbs.count) udl_free_urb_list(dev); + if (udl->cursor) + udl_cursor_free(udl->cursor); + udl_fbdev_cleanup(dev); udl_modeset_cleanup(dev); kfree(udl); diff --git a/drivers/gpu/drm/udl/udl_modeset.c b/drivers/gpu/drm/udl/udl_modeset.c index f3804b4..d464286 100644 --- a/drivers/gpu/drm/udl/udl_modeset.c +++ b/drivers/gpu/drm/udl/udl_modeset.c @@ -16,6 +16,7 @@ #include <drm/drm_crtc_helper.h> #include <drm/drm_plane_helper.h> #include "udl_drv.h" +#include "udl_cursor.h" /* * All DisplayLink bulk operations start with 0xAF, followed by specific code @@ -471,6 +472,48 @@ static void udl_crtc_commit(struct drm_crtc *crtc) udl_crtc_dpms(crtc, DRM_MODE_DPMS_ON); } +static int udl_crtc_cursor_set(struct drm_crtc *crtc, struct drm_file *file, + uint32_t handle, uint32_t width, uint32_t height) +{ + struct drm_device *dev = crtc->dev; + struct udl_device *udl = dev->dev_private; + int ret; + + mutex_lock(&dev->struct_mutex); + ret = udl_cursor_set(crtc, file, handle, width, height, udl->cursor); + mutex_unlock(&dev->struct_mutex); + + if (ret) { + DRM_ERROR("Failed to set UDL cursor\n"); + return ret; + } + + return udl_crtc_page_flip(crtc, NULL, NULL, 0); +} + +static int udl_crtc_cursor_move(struct drm_crtc *crtc, int x, int y) +{ + struct drm_device *dev = crtc->dev; + struct udl_device *udl = dev->dev_private; + int ret = 0; + + mutex_lock(&dev->struct_mutex); + if (!udl_cursor_enabled(udl->cursor)) + goto error; + ret = udl_cursor_move(crtc, x, y, udl->cursor); + if (ret) { + DRM_ERROR("Failed to move UDL cursor\n"); + goto error; + } + mutex_unlock(&dev->struct_mutex); + + return udl_crtc_page_flip(crtc, NULL, NULL, 0); + +error: + mutex_unlock(&dev->struct_mutex); + return ret; +} + static struct drm_crtc_helper_funcs udl_helper_funcs = { .dpms = udl_crtc_dpms, .mode_fixup = udl_crtc_mode_fixup, @@ -484,6 +527,8 @@ static const struct drm_crtc_funcs udl_crtc_funcs = { .set_config = drm_crtc_helper_set_config, .destroy = udl_crtc_destroy, .page_flip = udl_crtc_page_flip, + .cursor_set = udl_crtc_cursor_set, + .cursor_move = udl_crtc_cursor_move, }; static int udl_crtc_init(struct drm_device *dev) diff --git a/drivers/gpu/drm/udl/udl_transfer.c b/drivers/gpu/drm/udl/udl_transfer.c index 917dcb9..2f59603 100644 --- a/drivers/gpu/drm/udl/udl_transfer.c +++ b/drivers/gpu/drm/udl/udl_transfer.c @@ -17,6 +17,7 @@ #include <drm/drmP.h> #include "udl_drv.h" +#include "udl_cursor.h" #define MAX_CMD_PIXELS 255 @@ -92,6 +93,19 @@ static inline u16 get_pixel_val16(const uint8_t *pixel, int bpp) return pixel_val16; } +static inline u16 blend_alpha(const uint16_t pixel_val16, uint32_t blend_val32) +{ + uint32_t alpha = (blend_val32 >> 16); + uint32_t alpha_inv = 64 - alpha; + + return (((pixel_val16 & 0x1f) * alpha_inv + + (blend_val32 & 0x1f) * alpha) >> 6) | + ((((pixel_val16 & 0x7e0) * alpha_inv + + (blend_val32 & 0x7e0) * alpha) >> 6) & 0x7e0) | + ((((pixel_val16 & 0xf800) * alpha_inv + + (blend_val32 & 0xf800) * alpha) >> 6) & 0xf800); +} + /* * Render a command stream for an encoded horizontal line segment of pixels. * @@ -123,12 +137,16 @@ static void udl_compress_hline16( const u8 **pixel_start_ptr, const u8 *const pixel_end, uint32_t *device_address_ptr, + struct udl_cursor_hline *cursor_hline, uint8_t **command_buffer_ptr, const uint8_t *const cmd_buffer_end, int bpp) { const u8 *pixel = *pixel_start_ptr; uint32_t dev_addr = *device_address_ptr; uint8_t *cmd = *command_buffer_ptr; + const uint32_t *cursor_buf = cursor_hline ? cursor_hline->buffer : NULL; + int cursor_pos = cursor_buf ? cursor_hline->offset : 0; + int cursor_width = cursor_buf ? cursor_hline->width : 0; while ((pixel_end > pixel) && (cmd_buffer_end - MIN_RLX_CMD_BYTES > cmd)) { @@ -158,6 +176,11 @@ static void udl_compress_hline16( prefetch_range((void *) pixel, (cmd_pixel_end - pixel) * bpp); pixel_val16 = get_pixel_val16(pixel, bpp); + if (cursor_buf && cursor_pos >= 0 && + cursor_pos < cursor_width) { + pixel_val16 = blend_alpha(pixel_val16, + cursor_buf[cursor_pos]); + } while (pixel < cmd_pixel_end) { const u8 *const start = pixel; @@ -167,12 +190,19 @@ static void udl_compress_hline16( cmd += 2; pixel += bpp; + cursor_pos++; while (pixel < cmd_pixel_end) { pixel_val16 = get_pixel_val16(pixel, bpp); + if (cursor_buf && cursor_pos >= 0 && + cursor_pos < cursor_width) { + pixel_val16 = blend_alpha(pixel_val16, + cursor_buf[cursor_pos]); + } if (pixel_val16 != repeating_pixel_val16) break; pixel += bpp; + cursor_pos++; } if (unlikely(pixel > start + bpp)) { @@ -208,6 +238,8 @@ static void udl_compress_hline16( *command_buffer_ptr = cmd; *pixel_start_ptr = pixel; *device_address_ptr = dev_addr; + if (cursor_buf) + cursor_hline->offset = cursor_pos; return; } @@ -221,7 +253,7 @@ static void udl_compress_hline16( int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr, const char *front, char **urb_buf_ptr, u32 byte_offset, u32 device_byte_offset, - u32 byte_width, + u32 byte_width, struct udl_cursor_hline *cursor_hline, int *ident_ptr, int *sent_ptr) { const u8 *line_start, *line_end, *next_pixel; @@ -239,7 +271,7 @@ int udl_render_hline(struct drm_device *dev, int bpp, struct urb **urb_ptr, while (next_pixel < line_end) { udl_compress_hline16(&next_pixel, - line_end, &base16, + line_end, &base16, cursor_hline, (u8 **) &cmd, (u8 *) cmd_end, bpp); if (cmd >= cmd_end) {