Re: [Spice-devel] [PATCH] drm/qxl: Fix missing free_irq
On Tue, Nov 08, 2022 at 11:16:01PM +0800, Wei Li wrote:
> When doing "cat /proc/interrupts" after qxl.ko is unloaded, an oops occurs:
>
> BUG: unable to handle page fault for address: c0274769
> PGD 2a0d067 P4D 2a0d067 PUD 2a0f067 PMD 103f39067 PTE 0
> Oops: [#1] PREEMPT SMP PTI
> CPU: 6 PID: 246 Comm: cat Not tainted 6.1.0-rc2 #24
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1.1
> 04/01/2014
> RIP: 0010:string_nocheck+0x34/0x50
> Code: 66 85 c0 74 3c 83 e8 01 4c 8d 5c 07 01 31 c0 eb 19 49 39 fa 76 03 44 88
> 07 48 83 c7
> RSP: 0018:c9893bb8 EFLAGS: 00010046
> RAX: RBX: c9893c50 RCX: 0a00ff04
> RDX: c0274769 RSI: 888102812000 RDI: 88810281133e
> RBP: 888102812000 R08: 823fa5e6 R09: 0007
> R10: 888102812000 R11: 88820281133d R12: c0274769
> R13: 0a00ff04 R14: 0cc4 R15: 823276b4
> FS: 0214f8c0() GS:88842fd8() knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: c0274769 CR3: 0001025c4005 CR4: 00770ee0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> PKRU: 5554
> Call Trace:
>
> string+0x46/0x60
> vsnprintf+0x27a/0x4f0
> seq_vprintf+0x34/0x50
> seq_printf+0x53/0x70
> ? seq_read_iter+0x365/0x450
> show_interrupts+0x259/0x330
> seq_read_iter+0x2a3/0x450
> proc_reg_read_iter+0x47/0x70
> generic_file_splice_read+0x94/0x160
> splice_direct_to_actor+0xb0/0x230
> ? do_splice_direct+0xd0/0xd0
> do_splice_direct+0x8b/0xd0
> do_sendfile+0x345/0x4f0
> __x64_sys_sendfile64+0xa1/0xc0
> do_syscall_64+0x38/0x90
> entry_SYSCALL_64_after_hwframe+0x63/0xcd
> RIP: 0033:0x4bb0ce
> Code: c3 0f 1f 00 4c 89 d2 4c 89 c6 e9 bd fd ff ff 0f 1f 44 00 00 31 c0 c3 0f
> 1f 44 00 00
> RSP: 002b:7ffd99dc3fb8 EFLAGS: 0246 ORIG_RAX: 0028
> RAX: ffda RBX: 0100 RCX: 004bb0ce
> RDX: RSI: 0003 RDI: 0001
> RBP: 0001 R08: 0068f240 R09: 0100
> R10: 0100 R11: 0246 R12: 0003
> R13: 0001 R14: R15:
>
>
> It seems that qxl doesn't free the interrupt it requests during unload,
> fix this by adding the missing free_irq().
>
> Fixes: f64122c1f6ad ("drm: add new QXL driver. (v1.4)")
> Signed-off-by: Wei Li
Could we go right ahead and switch over to devm_request_irq? Or does that
not quite do the right thing here?
-Daniel
> ---
> drivers/gpu/drm/qxl/qxl_kms.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index dc3828db1991..d591084824de 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -283,6 +283,8 @@ int qxl_device_init(struct qxl_device *qdev,
> void qxl_device_fini(struct qxl_device *qdev)
> {
> int cur_idx;
> + struct drm_device *ddev = >ddev;
> + struct pci_dev *pdev = to_pci_dev(ddev->dev);
>
> /* check if qxl_device_init() was successful (gc_work is initialized
> last) */
> if (!qdev->gc_work.func)
> @@ -305,6 +307,7 @@ void qxl_device_fini(struct qxl_device *qdev)
> wait_event_timeout(qdev->release_event,
> atomic_read(>release_count) == 0,
> HZ);
> + free_irq(pdev->irq, ddev);
> flush_work(>gc_work);
> qxl_surf_evict(qdev);
> qxl_vram_evict(qdev);
> --
> 2.25.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [Spice-devel] [PATCH v2 1/8] drm: Disable the cursor plane on atomic contexts with virtualized drivers
On Mon, Jul 11, 2022 at 11:32:39PM -0400, Zack Rusin wrote:
> From: Zack Rusin
>
> Cursor planes on virtualized drivers have special meaning and require
> that the clients handle them in specific ways, e.g. the cursor plane
> should react to the mouse movement the way a mouse cursor would be
> expected to and the client is required to set hotspot properties on it
> in order for the mouse events to be routed correctly.
>
> This breaks the contract as specified by the "universal planes". Fix it
> by disabling the cursor planes on virtualized drivers while adding
> a foundation on top of which it's possible to special case mouse cursor
> planes for clients that want it.
>
> Disabling the cursor planes makes some kms compositors which were broken,
> e.g. Weston, fallback to software cursor which works fine or at least
> better than currently while having no effect on others, e.g. gnome-shell
> or kwin, which put virtualized drivers on a deny-list when running in
> atomic context to make them fallback to legacy kms and avoid this issue.
>
> Signed-off-by: Zack Rusin
> Fixes: 681e7ec73044 ("drm: Allow userspace to ask for universal plane list
> (v2)")
> Cc: # v5.4+
> Cc: Maarten Lankhorst
> Cc: Maxime Ripard
> Cc: Thomas Zimmermann
> Cc: David Airlie
> Cc: Daniel Vetter
> Cc: Dave Airlie
> Cc: Gerd Hoffmann
> Cc: Hans de Goede
> Cc: Gurchetan Singh
> Cc: Chia-I Wu
> Cc: dri-de...@lists.freedesktop.org
> Cc: virtualizat...@lists.linux-foundation.org
> Cc: spice-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/drm_plane.c | 11 +++
> drivers/gpu/drm/qxl/qxl_drv.c| 2 +-
> drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_drv.c | 3 ++-
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
> include/drm/drm_drv.h| 10 ++
> include/drm/drm_file.h | 12
> 7 files changed, 38 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
> index 726f2f163c26..e1e2a65c7119 100644
> --- a/drivers/gpu/drm/drm_plane.c
> +++ b/drivers/gpu/drm/drm_plane.c
> @@ -667,6 +667,17 @@ int drm_mode_getplane_res(struct drm_device *dev, void
> *data,
> !file_priv->universal_planes)
> continue;
>
> + /*
> + * Unless userspace supports virtual cursor plane
> + * then if we're running on virtual driver do not
> + * advertise cursor planes because they'll be broken
> + */
> + if (plane->type == DRM_PLANE_TYPE_CURSOR &&
> + drm_core_check_feature(dev, DRIVER_VIRTUAL) &&
> + file_priv->atomic &&
> + !file_priv->supports_virtual_cursor_plane)
> + continue;
> +
> if (drm_lease_held(file_priv, plane->base.id)) {
> if (count < plane_resp->count_planes &&
> put_user(plane->base.id, plane_ptr + count))
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 1cb6f0c224bb..0e4212e05caa 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -281,7 +281,7 @@ static const struct drm_ioctl_desc qxl_ioctls[] = {
> };
>
> static struct drm_driver qxl_driver = {
> - .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC,
> + .driver_features = DRIVER_GEM | DRIVER_MODESET | DRIVER_ATOMIC |
> DRIVER_VIRTUAL,
>
> .dumb_create = qxl_mode_dumb_create,
> .dumb_map_offset = drm_gem_ttm_dumb_map_offset,
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index f4f2bd79a7cb..84e75bcc3384 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -176,7 +176,7 @@ DEFINE_DRM_GEM_FOPS(vbox_fops);
>
> static const struct drm_driver driver = {
> .driver_features =
> - DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC,
> + DRIVER_MODESET | DRIVER_GEM | DRIVER_ATOMIC | DRIVER_VIRTUAL,
>
> .lastclose = drm_fb_helper_lastclose,
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c
> b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index 5f25a8d15464..3c5bb006159a 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -198,7 +198,8 @@ MODULE_AUTHOR("Alon Levy");
> DEFINE_DRM_GEM_FOPS(virtio_gpu_driver_fops);
>
> static const struct drm_driver driver = {
> - .driver_features = DRIVER_MODESET | DRIVER_GEM | DR
Re: [Spice-devel] [PATCH 4/4] drm/qxl: add drm_gem_plane_helper_prepare_fb
On Fri, Apr 29, 2022 at 03:42:30PM +0200, Christian König wrote: > We could need to wait for the pin to complete here. > > Signed-off-by: Christian König > Cc: Dave Airlie > Cc: Gerd Hoffmann > Cc: virtualizat...@lists.linux-foundation.org > Cc: spice-devel@lists.freedesktop.org Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/qxl/qxl_display.c | 8 +++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_display.c > b/drivers/gpu/drm/qxl/qxl_display.c > index 9a9c29b1d3e1..9a64fa4c7530 100644 > --- a/drivers/gpu/drm/qxl/qxl_display.c > +++ b/drivers/gpu/drm/qxl/qxl_display.c > @@ -34,6 +34,7 @@ > #include > #include > #include > +#include > > #include "qxl_drv.h" > #include "qxl_object.h" > @@ -829,6 +830,7 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane, > struct qxl_device *qdev = to_qxl(plane->dev); > struct drm_gem_object *obj; > struct qxl_bo *user_bo; > + int ret; > > if (!new_state->fb) > return 0; > @@ -852,7 +854,11 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane, > qxl_free_cursor(old_cursor_bo); > } > > - return qxl_bo_pin(user_bo); > + ret = qxl_bo_pin(user_bo); > + if (ret) > + return ret; > + > + return drm_gem_plane_helper_prepare_fb(plane, new_state); > } > > static void qxl_plane_cleanup_fb(struct drm_plane *plane, > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [Spice-devel] [PATCH 0/4] drm: Generic dumb_map_offset for TTM-based drivers
On Tue, Apr 06, 2021 at 10:29:38AM +0200, Thomas Zimmermann wrote: > The implementation of drm_driver.dumb_map_offset is the same for several > TTM-based drivers. Provide a common function in GEM-TTM helpers. Out of curiosity, why does this not fit for radeon/amdgpu? -Daniel > > Thomas Zimmermann (4): > drm/gem-ttm-helper: Provide helper for struct > drm_driver.dumb_map_offset > drm/vram-helper: Use drm_gem_ttm_dumb_map_offset() > drm/nouveau: Use drm_gem_ttm_dumb_map_offset() > drm/qxl: Use drm_gem_ttm_dumb_map_offset() > > drivers/gpu/drm/drm_gem_ttm_helper.c | 33 > drivers/gpu/drm/drm_gem_vram_helper.c | 48 --- > drivers/gpu/drm/nouveau/nouveau_display.c | 18 - > drivers/gpu/drm/nouveau/nouveau_display.h | 2 - > drivers/gpu/drm/nouveau/nouveau_drm.c | 3 +- > drivers/gpu/drm/qxl/qxl_drv.c | 3 +- > drivers/gpu/drm/qxl/qxl_drv.h | 3 -- > drivers/gpu/drm/qxl/qxl_dumb.c| 17 > drivers/gpu/drm/qxl/qxl_ioctl.c | 4 +- > drivers/gpu/drm/qxl/qxl_object.h | 5 --- > include/drm/drm_gem_ttm_helper.h | 5 ++- > include/drm/drm_gem_vram_helper.h | 7 +--- > 12 files changed, 45 insertions(+), 103 deletions(-) > > -- > 2.30.2 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 0/4] drm: Generic dumb_map_offset for TTM-based drivers
On Thu, Apr 08, 2021 at 01:34:03PM +0200, Thomas Zimmermann wrote: > Hi > > Am 08.04.21 um 13:16 schrieb Daniel Vetter: > > On Tue, Apr 06, 2021 at 10:29:38AM +0200, Thomas Zimmermann wrote: > > > The implementation of drm_driver.dumb_map_offset is the same for several > > > TTM-based drivers. Provide a common function in GEM-TTM helpers. > > > > Out of curiosity, why does this not fit for radeon/amdgpu? > > These drivers perform additional permission checks in their implementations. > > But those checks are also part of the actual mmap code. I want to propose a > patch to use the generic drm_gem_ttm_map_offset in amdgpu/radeon, and then > rely on mmap to fail if necessary. It might result in a longer discussion, > so that's for another patchset. Ah cool, makes sense. -Daniel > > Best regards > Thomas > > > -Daniel > > > > > > > > Thomas Zimmermann (4): > > >drm/gem-ttm-helper: Provide helper for struct > > > drm_driver.dumb_map_offset > > >drm/vram-helper: Use drm_gem_ttm_dumb_map_offset() > > >drm/nouveau: Use drm_gem_ttm_dumb_map_offset() > > >drm/qxl: Use drm_gem_ttm_dumb_map_offset() > > > > > > drivers/gpu/drm/drm_gem_ttm_helper.c | 33 > > > drivers/gpu/drm/drm_gem_vram_helper.c | 48 --- > > > drivers/gpu/drm/nouveau/nouveau_display.c | 18 - > > > drivers/gpu/drm/nouveau/nouveau_display.h | 2 - > > > drivers/gpu/drm/nouveau/nouveau_drm.c | 3 +- > > > drivers/gpu/drm/qxl/qxl_drv.c | 3 +- > > > drivers/gpu/drm/qxl/qxl_drv.h | 3 -- > > > drivers/gpu/drm/qxl/qxl_dumb.c| 17 > > > drivers/gpu/drm/qxl/qxl_ioctl.c | 4 +- > > > drivers/gpu/drm/qxl/qxl_object.h | 5 --- > > > include/drm/drm_gem_ttm_helper.h | 5 ++- > > > include/drm/drm_gem_vram_helper.h | 7 +--- > > > 12 files changed, 45 insertions(+), 103 deletions(-) > > > > > > -- > > > 2.30.2 > > > > > > > -- > Thomas Zimmermann > Graphics Driver Developer > SUSE Software Solutions Germany GmbH > Maxfeldstr. 5, 90409 Nürnberg, Germany > (HRB 36809, AG Nürnberg) > Geschäftsführer: Felix Imendörffer > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 5/5] drm/qxl: properly free qxl releases
On Tue, Jan 26, 2021 at 05:58:12PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/qxl/qxl_drv.h | 1 +
> drivers/gpu/drm/qxl/qxl_kms.c | 22 --
> drivers/gpu/drm/qxl/qxl_release.c | 2 ++
> 3 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 01354b43c413..1c57b587b6a7 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -214,6 +214,7 @@ struct qxl_device {
> spinlock_t release_lock;
> struct idr release_idr;
> uint32_trelease_seqno;
> + atomic_trelease_count;
> spinlock_t release_idr_lock;
> struct mutexasync_io_mutex;
> unsigned int last_sent_io_cmd;
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index 4a60a52ab62e..f177f72bfc12 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -25,6 +25,7 @@
>
> #include
> #include
> +#include
>
> #include
> #include
> @@ -286,8 +287,25 @@ int qxl_device_init(struct qxl_device *qdev,
>
> void qxl_device_fini(struct qxl_device *qdev)
> {
> - qxl_bo_unref(>current_release_bo[0]);
> - qxl_bo_unref(>current_release_bo[1]);
> + int cur_idx, try;
> +
> + for (cur_idx = 0; cur_idx < 3; cur_idx++) {
> + if (!qdev->current_release_bo[cur_idx])
> + continue;
> + qxl_bo_unpin(qdev->current_release_bo[cur_idx]);
> + qxl_bo_unref(>current_release_bo[cur_idx]);
> + qdev->current_release_bo_offset[cur_idx] = 0;
> + qdev->current_release_bo[cur_idx] = NULL;
> + }
> +
> + /*
> + * Ask host to release resources (+fill release ring),
> + * then wait for the release actually happening.
> + */
> + qxl_io_notify_oom(qdev);
> + for (try = 0; try < 20 && atomic_read(>release_count) > 0; try++)
> + msleep(20);
A bit icky, why not use a wait queue or something like that instead of
hand-rolling this? Not for perf reasons, just so it's a bit clear who
waits for whom and why.
-Daniel
> +
> qxl_gem_fini(qdev);
> qxl_bo_fini(qdev);
> flush_work(>gc_work);
> diff --git a/drivers/gpu/drm/qxl/qxl_release.c
> b/drivers/gpu/drm/qxl/qxl_release.c
> index 28013fd1f8ea..43a5436853b7 100644
> --- a/drivers/gpu/drm/qxl/qxl_release.c
> +++ b/drivers/gpu/drm/qxl/qxl_release.c
> @@ -196,6 +196,7 @@ qxl_release_free(struct qxl_device *qdev,
> qxl_release_free_list(release);
> kfree(release);
> }
> + atomic_dec(>release_count);
> }
>
> static int qxl_release_bo_alloc(struct qxl_device *qdev,
> @@ -344,6 +345,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
> unsigned long size,
> *rbo = NULL;
> return idr_ret;
> }
> + atomic_inc(>release_count);
>
> mutex_lock(>release_mutex);
> if (qdev->current_release_bo_offset[cur_idx] + 1 >=
> releases_per_bo[cur_idx]) {
> --
> 2.29.2
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 2/4] drm/qxl: unpin release objects
On Fri, Jan 22, 2021 at 2:35 PM Gerd Hoffmann wrote: > > On Fri, Jan 22, 2021 at 09:13:42AM +0100, Thomas Zimmermann wrote: > > Hi > > > > Am 20.01.21 um 12:12 schrieb Gerd Hoffmann: > > > Balances the qxl_create_bo(..., pinned=true, ...); > > > call in qxl_release_bo_alloc(). > > > > > > Signed-off-by: Gerd Hoffmann > > > --- > > > drivers/gpu/drm/qxl/qxl_release.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/gpu/drm/qxl/qxl_release.c > > > b/drivers/gpu/drm/qxl/qxl_release.c > > > index 0fcfc952d5e9..add979cba11b 100644 > > > --- a/drivers/gpu/drm/qxl/qxl_release.c > > > +++ b/drivers/gpu/drm/qxl/qxl_release.c > > > @@ -166,6 +166,7 @@ qxl_release_free_list(struct qxl_release *release) > > > entry = container_of(release->bos.next, > > > struct qxl_bo_list, tv.head); > > > bo = to_qxl_bo(entry->tv.bo); > > > + bo->tbo.pin_count = 0; /* ttm_bo_unpin(>tbo); */ > > > > This code looks like a workaround or a bug. > > > > AFAICT the only place with pre-pinned BO is qdev->dumb_shadow_bo. Can you > > remove the pinned flag entirely and handle pinning as part of > > dumb_shadow_bo's code. > > No, the release objects are pinned too, and they must be > pinned (qxl commands are in there, and references are > placed in the qxl rings, so allowing them to roam is > a non-starter). > > > if (pin_count) > > ttm_bo_unpin(); > > WARN_ON(pin_count); /* should always be 0 now */ > > Well, the pin_count is 1 at this point. > No need for the if(). > > Just calling ttm_bo_unpin() here makes lockdep unhappy. How does that one splat? But yeah if that's a problem should be explained in the comment. I'd then also only do a pin_count--; to make sure you can still catch other pin leaks if you have them. Setting it to 0 kinda defeats the warning. -Daniel > > Not calling ttm_bo_unpin() makes ttm_bo_release() throw > a WARN() because of the pin. > > Clearing pin_count (which is how ttm fixes things up > in the error path) works. > > I'm open to better ideas. > > take care, > Gerd > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 14/15] drm/vmwgfx: Remove references to struct drm_device.pdev
On Wed, Dec 2, 2020 at 4:37 PM Zack Rusin wrote: > > > > > On Dec 2, 2020, at 09:27, Thomas Zimmermann wrote: > > > > Hi > > > > Am 02.12.20 um 09:01 schrieb Thomas Zimmermann: > >> Hi > >> Am 30.11.20 um 21:59 schrieb Zack Rusin: > >>> > >>> > >>>> On Nov 24, 2020, at 06:38, Thomas Zimmermann wrote: > >>>> > >>>> Using struct drm_device.pdev is deprecated. Convert vmwgfx to struct > >>>> drm_device.dev. No functional changes. > >>>> > >>>> Signed-off-by: Thomas Zimmermann > >>>> Cc: Roland Scheidegger > >>>> --- > >>>> drivers/gpu/drm/vmwgfx/vmwgfx_cmdbuf.c | 8 > >>>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c| 27 +- > >>>> drivers/gpu/drm/vmwgfx/vmwgfx_fb.c | 2 +- > >>> > >>> Reviewed-by: Zack Rusin > >> Could you add this patch to the vmwgfx tree? > > > > AMD devs indicated that they'd prefer to merge the patchset trough > > drm-misc-next. If you're OK with that, I'd merge the vmwgfx patch through > > drm-misc-next as well. > > Sounds good. I’ll make sure to rebase our latest patch set on top of it when > it’s in. Thanks! btw if you want to avoid multi-tree coordination headaches, we can also manage vmwgfx in drm-misc and give you & Roland commit rights there. Up to you. There is some scripting involved for now (but I hope whenever we move to gitlab we could do the checks server-side): https://drm.pages.freedesktop.org/maintainer-tools/getting-started.html Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] drm/qxl: Remove fbcon acceleration leftovers
On Thu, Oct 29, 2020 at 02:33:47PM +0100, Daniel Vetter wrote:
> These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev
> emulation code").
>
> v2: Somehow these structs provided the struct qxl_device pre-decl,
> reorder the header to not anger compilers.
>
> Acked-by: Gerd Hoffmann
> Signed-off-by: Daniel Vetter
> Cc: Dave Airlie
> Cc: Gerd Hoffmann
> Cc: virtualizat...@lists.linux-foundation.org
> Cc: spice-devel@lists.freedesktop.org
Entire series applied to drm-misc-next.
-Daniel
> ---
> drivers/gpu/drm/qxl/qxl_drv.h | 18 ++
> 1 file changed, 2 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> index 3602e8b34189..6239626503ef 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.h
> +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> @@ -166,20 +166,6 @@ struct qxl_drm_image {
> struct list_head chunk_list;
> };
>
> -struct qxl_fb_image {
> - struct qxl_device *qdev;
> - uint32_t pseudo_palette[16];
> - struct fb_image fb_image;
> - uint32_t visual;
> -};
> -
> -struct qxl_draw_fill {
> - struct qxl_device *qdev;
> - struct qxl_rect rect;
> - uint32_t color;
> - uint16_t rop;
> -};
> -
> /*
> * Debugfs
> */
> @@ -188,8 +174,6 @@ struct qxl_debugfs {
> unsigned int num_files;
> };
>
> -int qxl_debugfs_fence_init(struct qxl_device *rdev);
> -
> struct qxl_device {
> struct drm_device ddev;
>
> @@ -271,6 +255,8 @@ struct qxl_device {
>
> #define to_qxl(dev) container_of(dev, struct qxl_device, ddev)
>
> +int qxl_debugfs_fence_init(struct qxl_device *rdev);
> +
> extern const struct drm_ioctl_desc qxl_ioctls[];
> extern int qxl_max_ioctl;
>
> --
> 2.28.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v5 09/10] dma-buf-map: Add memcpy and pointer-increment interfaces
On Thu, Nov 05, 2020 at 11:37:08AM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 05.11.20 um 11:07 schrieb Linus Walleij:
> > Overall I like this, just an inline question:
> >
> > On Tue, Oct 20, 2020 at 2:20 PM Thomas Zimmermann
> > wrote:
> >
> >> To do framebuffer updates, one needs memcpy from system memory and a
> >> pointer-increment function. Add both interfaces with documentation.
> >
> > (...)
> >> +/**
> >> + * dma_buf_map_memcpy_to - Memcpy into dma-buf mapping
> >> + * @dst: The dma-buf mapping structure
> >> + * @src: The source buffer
> >> + * @len: The number of byte in src
> >> + *
> >> + * Copies data into a dma-buf mapping. The source buffer is in system
> >> + * memory. Depending on the buffer's location, the helper picks the
> >> correct
> >> + * method of accessing the memory.
> >> + */
> >> +static inline void dma_buf_map_memcpy_to(struct dma_buf_map *dst, const
> >> void *src, size_t len)
> >> +{
> >> + if (dst->is_iomem)
> >> + memcpy_toio(dst->vaddr_iomem, src, len);
> >> + else
> >> + memcpy(dst->vaddr, src, len);
> >> +}
> >
> > Are these going to be really big memcpy() operations?
>
> Individually, each could be a scanline, so a few KiB. (4 bytes *
> horizontal resolution). Updating a full framebuffer can sum up to
> several MiB.
>
> >
> > Some platforms have DMA offload engines that can perform memcpy(),They
> > could be
> > drivers/dma, include/linux/dmaengine.h
> > especially if the CPU doesn't really need to touch the contents
> > and flush caches etc.
> > An example exist in some MTD drivers that move large quantities of
> > data off flash memory like this:
> > drivers/mtd/nand/raw/cadence-nand-controller.c
> >
> > Notice that DMAengine and DMAbuf does not have much in common,
> > the names can be deceiving.
> >
> > The value of this varies with the system architecture. It is not just
> > a question about performance but also about power and the CPU
> > being able to do other stuff in parallel for large transfers. So *when*
> > to use this facility to accelerate memcpy() is a delicate question.
> >
> > What I'm after here is if these can be really big, do we want
> > (in the long run, not now) open up to the idea to slot in
> > hardware-accelerated memcpy() here?
>
> We currently use this functionality for the graphical framebuffer
> console that most DRM drivers provide. It's non-accelerated and slow,
> but this has not been much of a problem so far.
>
> Within DRM, we're more interested in removing console code from drivers
> and going for the generic implementation.
>
> Most of the graphics HW allocates framebuffers from video RAM, system
> memory or CMA pools and does not really need these memcpys. Only a few
> systems with small video RAM require a shadow buffer, which we flush
> into VRAM as needed. Those might benefit.
>
> OTOH, off-loading memcpys to hardware sounds reasonable if we can hide
> it from the DRM code. I think it all depends on how invasive that change
> would be.
I wouldn't, all the additional locks this would pull in sound like
nightmare. And when an oops happens, this might be the only thing that
manages to get the oops to the user.
Unless someone really starts caring about fbcon acceleration I really
wouldn't bother. Ok maybe it also matters for fbdev, but the problem is
that the page fault intercepting alone is already expensive, so the only
real solution if you care about performance in that case is to use kms
natively, and use a dirty rectangle flip (or the DIRTY syscall).
And in there drivers should (and do) use any dma engines they have to
upload the frames already.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] drm/qxl: Remove fbcon acceleration leftovers
These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev
emulation code").
v2: Somehow these structs provided the struct qxl_device pre-decl,
reorder the header to not anger compilers.
Acked-by: Gerd Hoffmann
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_drv.h | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 3602e8b34189..6239626503ef 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -166,20 +166,6 @@ struct qxl_drm_image {
struct list_head chunk_list;
};
-struct qxl_fb_image {
- struct qxl_device *qdev;
- uint32_t pseudo_palette[16];
- struct fb_image fb_image;
- uint32_t visual;
-};
-
-struct qxl_draw_fill {
- struct qxl_device *qdev;
- struct qxl_rect rect;
- uint32_t color;
- uint16_t rop;
-};
-
/*
* Debugfs
*/
@@ -188,8 +174,6 @@ struct qxl_debugfs {
unsigned int num_files;
};
-int qxl_debugfs_fence_init(struct qxl_device *rdev);
-
struct qxl_device {
struct drm_device ddev;
@@ -271,6 +255,8 @@ struct qxl_device {
#define to_qxl(dev) container_of(dev, struct qxl_device, ddev)
+int qxl_debugfs_fence_init(struct qxl_device *rdev);
+
extern const struct drm_ioctl_desc qxl_ioctls[];
extern int qxl_max_ioctl;
--
2.28.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 3/3] drm/qxl: Remove fbcon acceleration leftovers
These are leftovers from 13aff184ed9f ("drm/qxl: remove dead qxl fbdev
emulation code").
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_drv.h | 14 --
1 file changed, 14 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 3602e8b34189..86eee66ecbad 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -166,20 +166,6 @@ struct qxl_drm_image {
struct list_head chunk_list;
};
-struct qxl_fb_image {
- struct qxl_device *qdev;
- uint32_t pseudo_palette[16];
- struct fb_image fb_image;
- uint32_t visual;
-};
-
-struct qxl_draw_fill {
- struct qxl_device *qdev;
- struct qxl_rect rect;
- uint32_t color;
- uint16_t rop;
-};
-
/*
* Debugfs
*/
--
2.28.0
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v5 08/10] drm/gem: Store client buffer mappings as struct dma_buf_map
On Tue, Oct 20, 2020 at 02:20:44PM +0200, Thomas Zimmermann wrote:
> Kernel DRM clients now store their framebuffer address in an instance
> of struct dma_buf_map. Depending on the buffer's location, the address
> refers to system or I/O memory.
>
> Callers of drm_client_buffer_vmap() receive a copy of the value in
> the call's supplied arguments. It can be accessed and modified with
> dma_buf_map interfaces.
>
> Signed-off-by: Thomas Zimmermann
> Reviewed-by: Daniel Vetter
> Tested-by: Sam Ravnborg
> ---
> drivers/gpu/drm/drm_client.c| 34 +++--
> drivers/gpu/drm/drm_fb_helper.c | 23 +-
> include/drm/drm_client.h| 7 ---
> 3 files changed, 38 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> index ac0082bed966..fe573acf1067 100644
> --- a/drivers/gpu/drm/drm_client.c
> +++ b/drivers/gpu/drm/drm_client.c
> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
> drm_client_buffer *buffer)
> {
> struct drm_device *dev = buffer->client->dev;
>
> - drm_gem_vunmap(buffer->gem, buffer->vaddr);
> + drm_gem_vunmap(buffer->gem, >map);
>
> if (buffer->gem)
> drm_gem_object_put(buffer->gem);
> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev *client,
> u32 width, u32 height, u
> /**
> * drm_client_buffer_vmap - Map DRM client buffer into address space
> * @buffer: DRM client buffer
> + * @map_copy: Returns the mapped memory's address
> *
> * This function maps a client buffer into kernel address space. If the
> - * buffer is already mapped, it returns the mapping's address.
> + * buffer is already mapped, it returns the existing mapping's address.
> *
> * Client buffer mappings are not ref'counted. Each call to
> * drm_client_buffer_vmap() should be followed by a call to
> * drm_client_buffer_vunmap(); or the client buffer should be mapped
> * throughout its lifetime.
> *
> + * The returned address is a copy of the internal value. In contrast to
> + * other vmap interfaces, you don't need it for the client's vunmap
> + * function. So you can modify it at will during blit and draw operations.
> + *
> * Returns:
> - * The mapped memory's address
> + * 0 on success, or a negative errno code otherwise.
> */
> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> +int
> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct dma_buf_map
> *map_copy)
> {
> - struct dma_buf_map map;
> + struct dma_buf_map *map = >map;
> int ret;
>
> - if (buffer->vaddr)
> - return buffer->vaddr;
> + if (dma_buf_map_is_set(map))
> + goto out;
>
> /*
>* FIXME: The dependency on GEM here isn't required, we could
> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct drm_client_buffer
> *buffer)
>* fd_install step out of the driver backend hooks, to make that
>* final step optional for internal users.
>*/
> - ret = drm_gem_vmap(buffer->gem, );
> + ret = drm_gem_vmap(buffer->gem, map);
> if (ret)
> - return ERR_PTR(ret);
> + return ret;
>
> - buffer->vaddr = map.vaddr;
> +out:
> + *map_copy = *map;
>
> - return map.vaddr;
> + return 0;
> }
> EXPORT_SYMBOL(drm_client_buffer_vmap);
>
> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> */
> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> {
> - struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
> + struct dma_buf_map *map = >map;
>
> - drm_gem_vunmap(buffer->gem, );
> - buffer->vaddr = NULL;
> + drm_gem_vunmap(buffer->gem, map);
> }
> EXPORT_SYMBOL(drm_client_buffer_vunmap);
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index c2f72bb6afb1..6212cd7cde1d 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -378,7 +378,7 @@ static void drm_fb_helper_dirty_blit_real(struct
> drm_fb_helper *fb_helper,
> unsigned int cpp = fb->format->cpp[0];
> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> void *src = fb_helper->fbdev->screen_buffer + offset;
> - void *dst = fb_helper->buffer->vaddr + offset;
> + void *dst = fb_helper->buffer->map.vaddr + offset;
> size_t len = (clip->x2 - clip->x1) * cpp;
> unsigned int y;
>
> @@ -400,7 +400,8 @@ static void drm_fb
Re: [Spice-devel] [PATCH v5 10/10] drm/fb_helper: Support framebuffers in I/O memory
On Tue, Oct 20, 2020 at 02:20:46PM +0200, Thomas Zimmermann wrote:
> At least sparc64 requires I/O-specific access to framebuffers. This
> patch updates the fbdev console accordingly.
>
> For drivers with direct access to the framebuffer memory, the callback
> functions in struct fb_ops test for the type of memory and call the rsp
> fb_sys_ of fb_cfb_ functions. Read and write operations are implemented
> internally by DRM's fbdev helper.
>
> For drivers that employ a shadow buffer, fbdev's blit function retrieves
> the framebuffer address as struct dma_buf_map, and uses dma_buf_map
> interfaces to access the buffer.
>
> The bochs driver on sparc64 uses a workaround to flag the framebuffer as
> I/O memory and avoid a HW exception. With the introduction of struct
> dma_buf_map, this is not required any longer. The patch removes the rsp
> code from both, bochs and fbdev.
>
> v5:
> * implement fb_read/fb_write internally (Daniel, Sam)
> v4:
> * move dma_buf_map changes into separate patch (Daniel)
> * TODO list: comment on fbdev updates (Daniel)
>
> Signed-off-by: Thomas Zimmermann
> Tested-by: Sam Ravnborg
> ---
> Documentation/gpu/todo.rst| 19 ++-
> drivers/gpu/drm/bochs/bochs_kms.c | 1 -
> drivers/gpu/drm/drm_fb_helper.c | 227 --
> include/drm/drm_mode_config.h | 12 --
> 4 files changed, 230 insertions(+), 29 deletions(-)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 7e6fc3c04add..638b7f704339 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -197,13 +197,28 @@ Convert drivers to use drm_fbdev_generic_setup()
>
>
> Most drivers can use drm_fbdev_generic_setup(). Driver have to implement
> -atomic modesetting and GEM vmap support. Current generic fbdev emulation
> -expects the framebuffer in system memory (or system-like memory).
> +atomic modesetting and GEM vmap support. Historically, generic fbdev
> emulation
> +expected the framebuffer in system memory or system-like memory. By employing
> +struct dma_buf_map, drivers with frambuffers in I/O memory can be supported
> +as well.
>
> Contact: Maintainer of the driver you plan to convert
>
> Level: Intermediate
>
> +Reimplement functions in drm_fbdev_fb_ops without fbdev
> +---
> +
> +A number of callback functions in drm_fbdev_fb_ops could benefit from
> +being rewritten without dependencies on the fbdev module. Some of the
> +helpers could further benefit from using struct dma_buf_map instead of
> +raw pointers.
> +
> +Contact: Thomas Zimmermann , Daniel Vetter
> +
> +Level: Advanced
> +
> +
> drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup
> -
>
> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
> b/drivers/gpu/drm/bochs/bochs_kms.c
> index 13d0d04c4457..853081d186d5 100644
> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
> bochs->dev->mode_config.preferred_depth = 24;
> bochs->dev->mode_config.prefer_shadow = 0;
> bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> - bochs->dev->mode_config.fbdev_use_iomem = true;
> bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
>
> bochs->dev->mode_config.funcs = _mode_funcs;
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 6212cd7cde1d..1d3180841778 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -372,24 +372,22 @@ static void drm_fb_helper_resume_worker(struct
> work_struct *work)
> }
>
> static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
> - struct drm_clip_rect *clip)
> + struct drm_clip_rect *clip,
> + struct dma_buf_map *dst)
> {
> struct drm_framebuffer *fb = fb_helper->fb;
> unsigned int cpp = fb->format->cpp[0];
> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> void *src = fb_helper->fbdev->screen_buffer + offset;
> - void *dst = fb_helper->buffer->map.vaddr + offset;
> size_t len = (clip->x2 - clip->x1) * cpp;
> unsigned int y;
>
> - for (y = clip->y1; y < clip->y2; y++) {
> - if (!fb_helper->dev->mode_config.fbdev_use_iomem)
>
Re: [Spice-devel] [PATCH v5 08/10] drm/gem: Store client buffer mappings as struct dma_buf_map
On Thu, Oct 22, 2020 at 11:18 AM Thomas Zimmermann wrote:
>
> Hi
>
> On 22.10.20 10:49, Daniel Vetter wrote:
> > On Tue, Oct 20, 2020 at 02:20:44PM +0200, Thomas Zimmermann wrote:
> >> Kernel DRM clients now store their framebuffer address in an instance
> >> of struct dma_buf_map. Depending on the buffer's location, the address
> >> refers to system or I/O memory.
> >>
> >> Callers of drm_client_buffer_vmap() receive a copy of the value in
> >> the call's supplied arguments. It can be accessed and modified with
> >> dma_buf_map interfaces.
> >>
> >> Signed-off-by: Thomas Zimmermann
> >> Reviewed-by: Daniel Vetter
> >> Tested-by: Sam Ravnborg
> >> ---
> >> drivers/gpu/drm/drm_client.c| 34 +++--
> >> drivers/gpu/drm/drm_fb_helper.c | 23 +-
> >> include/drm/drm_client.h| 7 ---
> >> 3 files changed, 38 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/drm_client.c b/drivers/gpu/drm/drm_client.c
> >> index ac0082bed966..fe573acf1067 100644
> >> --- a/drivers/gpu/drm/drm_client.c
> >> +++ b/drivers/gpu/drm/drm_client.c
> >> @@ -235,7 +235,7 @@ static void drm_client_buffer_delete(struct
> >> drm_client_buffer *buffer)
> >> {
> >> struct drm_device *dev = buffer->client->dev;
> >>
> >> -drm_gem_vunmap(buffer->gem, buffer->vaddr);
> >> +drm_gem_vunmap(buffer->gem, >map);
> >>
> >> if (buffer->gem)
> >> drm_gem_object_put(buffer->gem);
> >> @@ -291,25 +291,31 @@ drm_client_buffer_create(struct drm_client_dev
> >> *client, u32 width, u32 height, u
> >> /**
> >> * drm_client_buffer_vmap - Map DRM client buffer into address space
> >> * @buffer: DRM client buffer
> >> + * @map_copy: Returns the mapped memory's address
> >> *
> >> * This function maps a client buffer into kernel address space. If the
> >> - * buffer is already mapped, it returns the mapping's address.
> >> + * buffer is already mapped, it returns the existing mapping's address.
> >> *
> >> * Client buffer mappings are not ref'counted. Each call to
> >> * drm_client_buffer_vmap() should be followed by a call to
> >> * drm_client_buffer_vunmap(); or the client buffer should be mapped
> >> * throughout its lifetime.
> >> *
> >> + * The returned address is a copy of the internal value. In contrast to
> >> + * other vmap interfaces, you don't need it for the client's vunmap
> >> + * function. So you can modify it at will during blit and draw operations.
> >> + *
> >> * Returns:
> >> - * The mapped memory's address
> >> + * 0 on success, or a negative errno code otherwise.
> >> */
> >> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer)
> >> +int
> >> +drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct
> >> dma_buf_map *map_copy)
> >> {
> >> -struct dma_buf_map map;
> >> +struct dma_buf_map *map = >map;
> >> int ret;
> >>
> >> -if (buffer->vaddr)
> >> -return buffer->vaddr;
> >> +if (dma_buf_map_is_set(map))
> >> +goto out;
> >>
> >> /*
> >> * FIXME: The dependency on GEM here isn't required, we could
> >> @@ -319,13 +325,14 @@ void *drm_client_buffer_vmap(struct
> >> drm_client_buffer *buffer)
> >> * fd_install step out of the driver backend hooks, to make that
> >> * final step optional for internal users.
> >> */
> >> -ret = drm_gem_vmap(buffer->gem, );
> >> +ret = drm_gem_vmap(buffer->gem, map);
> >> if (ret)
> >> -return ERR_PTR(ret);
> >> +return ret;
> >>
> >> -buffer->vaddr = map.vaddr;
> >> +out:
> >> +*map_copy = *map;
> >>
> >> -return map.vaddr;
> >> +return 0;
> >> }
> >> EXPORT_SYMBOL(drm_client_buffer_vmap);
> >>
> >> @@ -339,10 +346,9 @@ EXPORT_SYMBOL(drm_client_buffer_vmap);
> >> */
> >> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer)
> >> {
> >> -struct dma_buf_map map = DMA_BUF_MAP_INIT_VADDR(buffer->vaddr);
> >> +struct dma_buf_map *map = >map;
> >>
> >
Re: [Spice-devel] [PATCH v5 10/10] drm/fb_helper: Support framebuffers in I/O memory
On Thu, Oct 22, 2020 at 10:37:56AM +0200, Thomas Zimmermann wrote: > Hi > > On 22.10.20 10:05, Daniel Vetter wrote: > > On Tue, Oct 20, 2020 at 02:20:46PM +0200, Thomas Zimmermann wrote: > >> At least sparc64 requires I/O-specific access to framebuffers. This > >> patch updates the fbdev console accordingly. > >> > >> For drivers with direct access to the framebuffer memory, the callback > >> functions in struct fb_ops test for the type of memory and call the rsp > >> fb_sys_ of fb_cfb_ functions. Read and write operations are implemented > >> internally by DRM's fbdev helper. > >> > >> For drivers that employ a shadow buffer, fbdev's blit function retrieves > >> the framebuffer address as struct dma_buf_map, and uses dma_buf_map > >> interfaces to access the buffer. > >> > >> The bochs driver on sparc64 uses a workaround to flag the framebuffer as > >> I/O memory and avoid a HW exception. With the introduction of struct > >> dma_buf_map, this is not required any longer. The patch removes the rsp > >> code from both, bochs and fbdev. > >> > >> v5: > >>* implement fb_read/fb_write internally (Daniel, Sam) > >> v4: > >>* move dma_buf_map changes into separate patch (Daniel) > >>* TODO list: comment on fbdev updates (Daniel) > >> > >> Signed-off-by: Thomas Zimmermann > >> Tested-by: Sam Ravnborg > >> --- > >> Documentation/gpu/todo.rst| 19 ++- > >> drivers/gpu/drm/bochs/bochs_kms.c | 1 - > >> drivers/gpu/drm/drm_fb_helper.c | 227 -- > >> include/drm/drm_mode_config.h | 12 -- > >> 4 files changed, 230 insertions(+), 29 deletions(-) > >> > >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > >> index 7e6fc3c04add..638b7f704339 100644 > >> --- a/Documentation/gpu/todo.rst > >> +++ b/Documentation/gpu/todo.rst > >> @@ -197,13 +197,28 @@ Convert drivers to use drm_fbdev_generic_setup() > >> > >> > >> Most drivers can use drm_fbdev_generic_setup(). Driver have to implement > >> -atomic modesetting and GEM vmap support. Current generic fbdev emulation > >> -expects the framebuffer in system memory (or system-like memory). > >> +atomic modesetting and GEM vmap support. Historically, generic fbdev > >> emulation > >> +expected the framebuffer in system memory or system-like memory. By > >> employing > >> +struct dma_buf_map, drivers with frambuffers in I/O memory can be > >> supported > >> +as well. > >> > >> Contact: Maintainer of the driver you plan to convert > >> > >> Level: Intermediate > >> > >> +Reimplement functions in drm_fbdev_fb_ops without fbdev > >> +--- > >> + > >> +A number of callback functions in drm_fbdev_fb_ops could benefit from > >> +being rewritten without dependencies on the fbdev module. Some of the > >> +helpers could further benefit from using struct dma_buf_map instead of > >> +raw pointers. > >> + > >> +Contact: Thomas Zimmermann , Daniel Vetter > >> + > >> +Level: Advanced > >> + > >> + > >> drm_framebuffer_funcs and drm_mode_config_funcs.fb_create cleanup > >> - > >> > >> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c > >> b/drivers/gpu/drm/bochs/bochs_kms.c > >> index 13d0d04c4457..853081d186d5 100644 > >> --- a/drivers/gpu/drm/bochs/bochs_kms.c > >> +++ b/drivers/gpu/drm/bochs/bochs_kms.c > >> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs) > >>bochs->dev->mode_config.preferred_depth = 24; > >>bochs->dev->mode_config.prefer_shadow = 0; > >>bochs->dev->mode_config.prefer_shadow_fbdev = 1; > >> - bochs->dev->mode_config.fbdev_use_iomem = true; > >>bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true; > >> > >>bochs->dev->mode_config.funcs = _mode_funcs; > >> diff --git a/drivers/gpu/drm/drm_fb_helper.c > >> b/drivers/gpu/drm/drm_fb_helper.c > >> index 6212cd7cde1d..1d3180841778 100644 > >> --- a/drivers/gpu/drm/drm_fb_helper.c > >> +++ b/drivers/gpu/drm/drm_fb_helper.c > >> @@ -372,24 +372,22 @@ static void
Re: [Spice-devel] [PATCH v4 05/10] drm/ttm: Add vmap/vunmap to TTM and TTM GEM helpers
dma_buf_map_clear(map);
> > > > +
> > > > + ttm_mem_io_free(bo->bdev, >mem);
> > > > +}
> > > > +EXPORT_SYMBOL(ttm_bo_vunmap);
> > > > +
> > > > static int ttm_bo_wait_free_node(struct ttm_buffer_object *bo,
> > > > bool dst_use_tt)
> > > > {
> > > > diff --git a/include/drm/drm_gem_ttm_helper.h
> > > > b/include/drm/drm_gem_ttm_helper.h
> > > > index 118cef76f84f..7c6d874910b8 100644
> > > > --- a/include/drm/drm_gem_ttm_helper.h
> > > > +++ b/include/drm/drm_gem_ttm_helper.h
> > > > @@ -10,11 +10,17 @@
> > > > #include
> > > > #include
> > > > +struct dma_buf_map;
> > > > +
> > > > #define drm_gem_ttm_of_gem(gem_obj) \
> > > > container_of(gem_obj, struct ttm_buffer_object, base)
> > > > void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int
> > > > indent,
> > > > const struct drm_gem_object *gem);
> > > > +int drm_gem_ttm_vmap(struct drm_gem_object *gem,
> > > > + struct dma_buf_map *map);
> > > > +void drm_gem_ttm_vunmap(struct drm_gem_object *gem,
> > > > + struct dma_buf_map *map);
> > > > int drm_gem_ttm_mmap(struct drm_gem_object *gem,
> > > > struct vm_area_struct *vma);
> > > > diff --git a/include/drm/ttm/ttm_bo_api.h
> > > > b/include/drm/ttm/ttm_bo_api.h
> > > > index 37102e45e496..2c59a785374c 100644
> > > > --- a/include/drm/ttm/ttm_bo_api.h
> > > > +++ b/include/drm/ttm/ttm_bo_api.h
> > > > @@ -48,6 +48,8 @@ struct ttm_bo_global;
> > > > struct ttm_bo_device;
> > > > +struct dma_buf_map;
> > > > +
> > > > struct drm_mm_node;
> > > > struct ttm_placement;
> > > > @@ -494,6 +496,32 @@ int ttm_bo_kmap(struct ttm_buffer_object *bo,
> > > > unsigned long start_page,
> > > > */
> > > > void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map);
> > > > +/**
> > > > + * ttm_bo_vmap
> > > > + *
> > > > + * @bo: The buffer object.
> > > > + * @map: pointer to a struct dma_buf_map representing the map.
> > > > + *
> > > > + * Sets up a kernel virtual mapping, using ioremap or vmap to the
> > > > + * data in the buffer object. The parameter @map returns the virtual
> > > > + * address as struct dma_buf_map. Unmap the buffer with
> > > > ttm_bo_vunmap().
> > > > + *
> > > > + * Returns
> > > > + * -ENOMEM: Out of memory.
> > > > + * -EINVAL: Invalid range.
> > > > + */
> > > > +int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map);
> > > > +
> > > > +/**
> > > > + * ttm_bo_vunmap
> > > > + *
> > > > + * @bo: The buffer object.
> > > > + * @map: Object describing the map to unmap.
> > > > + *
> > > > + * Unmaps a kernel map set up by ttm_bo_vmap().
> > > > + */
> > > > +void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct dma_buf_map
> > > > *map);
> > > > +
> > > > /**
> > > > * ttm_bo_mmap_obj - mmap memory backed by a ttm buffer object.
> > > > *
> > > > diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> > > > index fd1aba545fdf..2e8bbecb5091 100644
> > > > --- a/include/linux/dma-buf-map.h
> > > > +++ b/include/linux/dma-buf-map.h
> > > > @@ -45,6 +45,12 @@
> > > > *
> > > > * dma_buf_map_set_vaddr( 0xdeadbeaf);
> > > > *
> > > > + * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem().
> > > > + *
> > > > + * .. code-block:: c
> > > > + *
> > > > + * dma_buf_map_set_vaddr_iomem( 0xdeadbeaf);
> > > > + *
> > > > * Test if a mapping is valid with either dma_buf_map_is_set() or
> > > > * dma_buf_map_is_null().
> > > > *
> > > > @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct
> > > > dma_buf_map *map, void *vaddr)
> > > > map->is_iomem = false;
> > > > }
> > > > +/**
> > > > + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to
> > > > an address in I/O memory
> > > > + * @map: The dma-buf mapping structure
> > > > + * @vaddr_iomem: An I/O-memory address
> > > > + *
> > > > + * Sets the address and the I/O-memory flag.
> > > > + */
> > > > +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map,
> > > > + void __iomem *vaddr_iomem)
> > > > +{
> > > > + map->vaddr_iomem = vaddr_iomem;
> > > > + map->is_iomem = true;
> > > > +}
> > > > +
> > > > /**
> > > > * dma_buf_map_is_equal - Compares two dma-buf mapping structures
> > > > for equality
> > > > * @lhs: The dma-buf mapping structure
> > > ___
> > > dri-devel mailing list
> > > dri-de...@lists.freedesktop.org
> > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-develdata=04%7C01%7Cchristian.koenig%40amd.com%7C07bc68af3c6440b5be8d08d8740e9b32%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637386953433558595%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000sdata=RlGCmjzyZERvqfnl4kA1bEHez5bkLf3F9OlKi2ybDAM%3Dreserved=0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 05/10] drm/ttm: Add vmap/vunmap to TTM and TTM GEM helpers
On Thu, Oct 15, 2020 at 04:08:13PM +0200, Christian König wrote:
> Am 15.10.20 um 14:38 schrieb Thomas Zimmermann:
> > The new functions ttm_bo_{vmap,vunmap}() map and unmap a TTM BO in kernel
> > address space. The mapping's address is returned as struct dma_buf_map.
> > Each function is a simplified version of TTM's existing kmap code. Both
> > functions respect the memory's location ani/or writecombine flags.
> >
> > On top TTM's functions, GEM TTM helpers got drm_gem_ttm_{vmap,vunmap}(),
> > two helpers that convert a GEM object into the TTM BO and forward the call
> > to TTM's vmap/vunmap. These helpers can be dropped into the rsp GEM object
> > callbacks.
> >
> > v4:
> > * drop ttm_kmap_obj_to_dma_buf() in favor of vmap helpers (Daniel,
> > Christian)
>
> Bunch of minor comments below, but over all look very solid to me.
Yeah I think just duplicating the ttm bo map stuff for vmap is indeed the
cleanest. And then we can maybe push the combinatorial monster into
vmwgfx, which I think is the only user after this series. Or perhaps a
dedicated set of helpers to map an invidual page (again using the
dma_buf_map stuff).
I'll let Christian with the details, but at a high level this is
definitely
Acked-by: Daniel Vetter
Thanks a lot for doing all this.
-Daniel
>
> >
> > Signed-off-by: Thomas Zimmermann
> > ---
> > drivers/gpu/drm/drm_gem_ttm_helper.c | 38 +++
> > drivers/gpu/drm/ttm/ttm_bo_util.c| 72
> > include/drm/drm_gem_ttm_helper.h | 6 +++
> > include/drm/ttm/ttm_bo_api.h | 28 +++
> > include/linux/dma-buf-map.h | 20
> > 5 files changed, 164 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
> > b/drivers/gpu/drm/drm_gem_ttm_helper.c
> > index 0e4fb9ba43ad..db4c14d78a30 100644
> > --- a/drivers/gpu/drm/drm_gem_ttm_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
> > @@ -49,6 +49,44 @@ void drm_gem_ttm_print_info(struct drm_printer *p,
> > unsigned int indent,
> > }
> > EXPORT_SYMBOL(drm_gem_ttm_print_info);
> > +/**
> > + * drm_gem_ttm_vmap() - vmap _buffer_object
> > + * @gem: GEM object.
> > + * @map: [out] returns the dma-buf mapping.
> > + *
> > + * Maps a GEM object with ttm_bo_vmap(). This function can be used as
> > + * _gem_object_funcs.vmap callback.
> > + *
> > + * Returns:
> > + * 0 on success, or a negative errno code otherwise.
> > + */
> > +int drm_gem_ttm_vmap(struct drm_gem_object *gem,
> > +struct dma_buf_map *map)
> > +{
> > + struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
> > +
> > + return ttm_bo_vmap(bo, map);
> > +
> > +}
> > +EXPORT_SYMBOL(drm_gem_ttm_vmap);
> > +
> > +/**
> > + * drm_gem_ttm_vunmap() - vunmap _buffer_object
> > + * @gem: GEM object.
> > + * @map: dma-buf mapping.
> > + *
> > + * Unmaps a GEM object with ttm_bo_vunmap(). This function can be used as
> > + * _gem_object_funcs.vmap callback.
> > + */
> > +void drm_gem_ttm_vunmap(struct drm_gem_object *gem,
> > + struct dma_buf_map *map)
> > +{
> > + struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
> > +
> > + ttm_bo_vunmap(bo, map);
> > +}
> > +EXPORT_SYMBOL(drm_gem_ttm_vunmap);
> > +
> > /**
> >* drm_gem_ttm_mmap() - mmap _buffer_object
> >* @gem: GEM object.
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > index bdee4df1f3f2..80c42c774c7d 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo_util.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo_util.c
> > @@ -32,6 +32,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -526,6 +527,77 @@ void ttm_bo_kunmap(struct ttm_bo_kmap_obj *map)
> > }
> > EXPORT_SYMBOL(ttm_bo_kunmap);
> > +int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map)
> > +{
> > + struct ttm_resource *mem = >mem;
> > + int ret;
> > +
> > + ret = ttm_mem_io_reserve(bo->bdev, mem);
> > + if (ret)
> > + return ret;
> > +
> > + if (mem->bus.is_iomem) {
> > + void __iomem *vaddr_iomem;
> > + unsigned long size = bo->num_pages << PAGE_SHIFT;
>
> Please use uint64_t here and make sure to cast bo->num_pages before
> shifting.
>
> We have an unit tests of allocating a 8GB BO and
Re: [Spice-devel] [PATCH v3 6/7] drm/fb_helper: Support framebuffers in I/O memory
On Thu, Oct 8, 2020 at 11:25 AM Thomas Zimmermann wrote:
>
> Hi
>
> Am 02.10.20 um 20:44 schrieb Daniel Vetter:
> > On Fri, Oct 2, 2020 at 8:05 PM Daniel Vetter wrote:
> >>
> >> On Tue, Sep 29, 2020 at 05:14:36PM +0200, Thomas Zimmermann wrote:
> >>> At least sparc64 requires I/O-specific access to framebuffers. This
> >>> patch updates the fbdev console accordingly.
> >>>
> >>> For drivers with direct access to the framebuffer memory, the callback
> >>> functions in struct fb_ops test for the type of memory and call the rsp
> >>> fb_sys_ of fb_cfb_ functions.
> >>>
> >>> For drivers that employ a shadow buffer, fbdev's blit function retrieves
> >>> the framebuffer address as struct dma_buf_map, and uses dma_buf_map
> >>> interfaces to access the buffer.
> >>>
> >>> The bochs driver on sparc64 uses a workaround to flag the framebuffer as
> >>> I/O memory and avoid a HW exception. With the introduction of struct
> >>> dma_buf_map, this is not required any longer. The patch removes the rsp
> >>> code from both, bochs and fbdev.
> >>>
> >>> Signed-off-by: Thomas Zimmermann
> >
> > Argh, I accidentally hit send before finishing this ...
> >
> >>> ---
> >>> drivers/gpu/drm/bochs/bochs_kms.c | 1 -
> >>> drivers/gpu/drm/drm_fb_helper.c | 217 --
> >>> include/drm/drm_mode_config.h | 12 --
> >>> include/linux/dma-buf-map.h | 72 --
> >>> 4 files changed, 265 insertions(+), 37 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
> >>> b/drivers/gpu/drm/bochs/bochs_kms.c
> >>> index 13d0d04c4457..853081d186d5 100644
> >>> --- a/drivers/gpu/drm/bochs/bochs_kms.c
> >>> +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> >>> @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
> >>> bochs->dev->mode_config.preferred_depth = 24;
> >>> bochs->dev->mode_config.prefer_shadow = 0;
> >>> bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> >>> - bochs->dev->mode_config.fbdev_use_iomem = true;
> >>> bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
> >>>
> >>> bochs->dev->mode_config.funcs = _mode_funcs;
> >>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> >>> b/drivers/gpu/drm/drm_fb_helper.c
> >>> index 343a292f2c7c..f345a314a437 100644
> >>> --- a/drivers/gpu/drm/drm_fb_helper.c
> >>> +++ b/drivers/gpu/drm/drm_fb_helper.c
> >>> @@ -388,24 +388,22 @@ static void drm_fb_helper_resume_worker(struct
> >>> work_struct *work)
> >>> }
> >>>
> >>> static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper
> >>> *fb_helper,
> >>> - struct drm_clip_rect *clip)
> >>> + struct drm_clip_rect *clip,
> >>> + struct dma_buf_map *dst)
> >>> {
> >>> struct drm_framebuffer *fb = fb_helper->fb;
> >>> unsigned int cpp = fb->format->cpp[0];
> >>> size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> >>> void *src = fb_helper->fbdev->screen_buffer + offset;
> >>> - void *dst = fb_helper->buffer->map.vaddr + offset;
> >>> size_t len = (clip->x2 - clip->x1) * cpp;
> >>> unsigned int y;
> >>>
> >>> - for (y = clip->y1; y < clip->y2; y++) {
> >>> - if (!fb_helper->dev->mode_config.fbdev_use_iomem)
> >>> - memcpy(dst, src, len);
> >>> - else
> >>> - memcpy_toio((void __iomem *)dst, src, len);
> >>> + dma_buf_map_incr(dst, offset); /* go to first pixel within clip
> >>> rect */
> >>>
> >>> + for (y = clip->y1; y < clip->y2; y++) {
> >>> + dma_buf_map_memcpy_to(dst, src, len);
> >>> + dma_buf_map_incr(dst, fb->pitches[0]);
> >>> src += fb->pitches[0];
> >>> - dst += fb->pitches[0];
> >>> }
> >>> }
> >>>
> >>> @@ -433,8 +431,9 @@ static void drm_fb_helper_d
Re: [Spice-devel] [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Wed, Oct 7, 2020 at 3:25 PM Christian König wrote: > > Am 07.10.20 um 15:20 schrieb Thomas Zimmermann: > > Hi > > > > Am 07.10.20 um 15:10 schrieb Daniel Vetter: > >> On Wed, Oct 7, 2020 at 2:57 PM Thomas Zimmermann > >> wrote: > >>> Hi > >>> > >>> Am 02.10.20 um 11:58 schrieb Daniel Vetter: > >>>> On Wed, Sep 30, 2020 at 02:51:46PM +0200, Daniel Vetter wrote: > >>>>> On Wed, Sep 30, 2020 at 2:34 PM Christian König > >>>>> wrote: > >>>>>> Am 30.09.20 um 11:47 schrieb Daniel Vetter: > >>>>>>> On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote: > >>>>>>>> Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: > >>>>>>>>> Hi > >>>>>>>>> > >>>>>>>>> Am 30.09.20 um 10:05 schrieb Christian König: > >>>>>>>>>> Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: > >>>>>>>>>>> Hi Christian > >>>>>>>>>>> > >>>>>>>>>>> Am 29.09.20 um 17:35 schrieb Christian König: > >>>>>>>>>>>> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: > >>>>>>>>>>>>> The new helper ttm_kmap_obj_to_dma_buf() extracts address and > >>>>>>>>>>>>> location > >>>>>>>>>>>>> from and instance of TTM's kmap_obj and initializes struct > >>>>>>>>>>>>> dma_buf_map > >>>>>>>>>>>>> with these values. Helpful for TTM-based drivers. > >>>>>>>>>>>> We could completely drop that if we use the same structure > >>>>>>>>>>>> inside TTM as > >>>>>>>>>>>> well. > >>>>>>>>>>>> > >>>>>>>>>>>> Additional to that which driver is going to use this? > >>>>>>>>>>> As Daniel mentioned, it's in patch 3. The TTM-based drivers will > >>>>>>>>>>> retrieve the pointer via this function. > >>>>>>>>>>> > >>>>>>>>>>> I do want to see all that being more tightly integrated into TTM, > >>>>>>>>>>> but > >>>>>>>>>>> not in this series. This one is about fixing the bochs-on-sparc64 > >>>>>>>>>>> problem for good. Patch 7 adds an update to TTM to the DRM TODO > >>>>>>>>>>> list. > >>>>>>>>>> I should have asked which driver you try to fix here :) > >>>>>>>>>> > >>>>>>>>>> In this case just keep the function inside bochs and only fix it > >>>>>>>>>> there. > >>>>>>>>>> > >>>>>>>>>> All other drivers can be fixed when we generally pump this through > >>>>>>>>>> TTM. > >>>>>>>>> Did you take a look at patch 3? This function will be used by VRAM > >>>>>>>>> helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, > >>>>>>>>> we > >>>>>>>>> have to duplicate the functionality in each if these drivers. Bochs > >>>>>>>>> itself uses VRAM helpers and doesn't touch the function directly. > >>>>>>>> Ah, ok can we have that then only in the VRAM helpers? > >>>>>>>> > >>>>>>>> Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj > >>>>>>>> directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. > >>>>>>>> > >>>>>>>> What I want to avoid is to have another conversion function in TTM > >>>>>>>> because > >>>>>>>> what happens here is that we already convert from ttm_bus_placement > >>>>>>>> to > >>>>>>>> ttm_bo_kmap_obj and then to dma_buf_map. > >>>>>>> Hm I'm not really seeing how that helps with a gradual conversion of > >>>>>>> everything over to dma_buf_map and assorted helpers for access? > >>>>&
Re: [Spice-devel] [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Wed, Oct 7, 2020 at 2:57 PM Thomas Zimmermann wrote: > > Hi > > Am 02.10.20 um 11:58 schrieb Daniel Vetter: > > On Wed, Sep 30, 2020 at 02:51:46PM +0200, Daniel Vetter wrote: > >> On Wed, Sep 30, 2020 at 2:34 PM Christian König > >> wrote: > >>> > >>> Am 30.09.20 um 11:47 schrieb Daniel Vetter: > >>>> On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote: > >>>>> Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: > >>>>>> Hi > >>>>>> > >>>>>> Am 30.09.20 um 10:05 schrieb Christian König: > >>>>>>> Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: > >>>>>>>> Hi Christian > >>>>>>>> > >>>>>>>> Am 29.09.20 um 17:35 schrieb Christian König: > >>>>>>>>> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: > >>>>>>>>>> The new helper ttm_kmap_obj_to_dma_buf() extracts address and > >>>>>>>>>> location > >>>>>>>>>> from and instance of TTM's kmap_obj and initializes struct > >>>>>>>>>> dma_buf_map > >>>>>>>>>> with these values. Helpful for TTM-based drivers. > >>>>>>>>> We could completely drop that if we use the same structure inside > >>>>>>>>> TTM as > >>>>>>>>> well. > >>>>>>>>> > >>>>>>>>> Additional to that which driver is going to use this? > >>>>>>>> As Daniel mentioned, it's in patch 3. The TTM-based drivers will > >>>>>>>> retrieve the pointer via this function. > >>>>>>>> > >>>>>>>> I do want to see all that being more tightly integrated into TTM, but > >>>>>>>> not in this series. This one is about fixing the bochs-on-sparc64 > >>>>>>>> problem for good. Patch 7 adds an update to TTM to the DRM TODO list. > >>>>>>> I should have asked which driver you try to fix here :) > >>>>>>> > >>>>>>> In this case just keep the function inside bochs and only fix it > >>>>>>> there. > >>>>>>> > >>>>>>> All other drivers can be fixed when we generally pump this through > >>>>>>> TTM. > >>>>>> Did you take a look at patch 3? This function will be used by VRAM > >>>>>> helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we > >>>>>> have to duplicate the functionality in each if these drivers. Bochs > >>>>>> itself uses VRAM helpers and doesn't touch the function directly. > >>>>> Ah, ok can we have that then only in the VRAM helpers? > >>>>> > >>>>> Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj > >>>>> directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. > >>>>> > >>>>> What I want to avoid is to have another conversion function in TTM > >>>>> because > >>>>> what happens here is that we already convert from ttm_bus_placement to > >>>>> ttm_bo_kmap_obj and then to dma_buf_map. > >>>> Hm I'm not really seeing how that helps with a gradual conversion of > >>>> everything over to dma_buf_map and assorted helpers for access? There's > >>>> too many places in ttm drivers where is_iomem and related stuff is used > >>>> to > >>>> be able to convert it all in one go. An intermediate state with a bunch > >>>> of > >>>> conversions seems fairly unavoidable to me. > >>> > >>> Fair enough. I would just have started bottom up and not top down. > >>> > >>> Anyway feel free to go ahead with this approach as long as we can remove > >>> the new function again when we clean that stuff up for good. > >> > >> Yeah I guess bottom up would make more sense as a refactoring. But the > >> main motivation to land this here is to fix the __mmio vs normal > >> memory confusion in the fbdev emulation helpers for sparc (and > >> anything else that needs this). Hence the top down approach for > >> rolling this out. > > > > Ok I started reviewing this a bit more in-depth, and I think this is
Re: [Spice-devel] [PATCH v3 6/7] drm/fb_helper: Support framebuffers in I/O memory
On Fri, Oct 2, 2020 at 8:05 PM Daniel Vetter wrote:
>
> On Tue, Sep 29, 2020 at 05:14:36PM +0200, Thomas Zimmermann wrote:
> > At least sparc64 requires I/O-specific access to framebuffers. This
> > patch updates the fbdev console accordingly.
> >
> > For drivers with direct access to the framebuffer memory, the callback
> > functions in struct fb_ops test for the type of memory and call the rsp
> > fb_sys_ of fb_cfb_ functions.
> >
> > For drivers that employ a shadow buffer, fbdev's blit function retrieves
> > the framebuffer address as struct dma_buf_map, and uses dma_buf_map
> > interfaces to access the buffer.
> >
> > The bochs driver on sparc64 uses a workaround to flag the framebuffer as
> > I/O memory and avoid a HW exception. With the introduction of struct
> > dma_buf_map, this is not required any longer. The patch removes the rsp
> > code from both, bochs and fbdev.
> >
> > Signed-off-by: Thomas Zimmermann
Argh, I accidentally hit send before finishing this ...
> > ---
> > drivers/gpu/drm/bochs/bochs_kms.c | 1 -
> > drivers/gpu/drm/drm_fb_helper.c | 217 --
> > include/drm/drm_mode_config.h | 12 --
> > include/linux/dma-buf-map.h | 72 --
> > 4 files changed, 265 insertions(+), 37 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/bochs/bochs_kms.c
> > b/drivers/gpu/drm/bochs/bochs_kms.c
> > index 13d0d04c4457..853081d186d5 100644
> > --- a/drivers/gpu/drm/bochs/bochs_kms.c
> > +++ b/drivers/gpu/drm/bochs/bochs_kms.c
> > @@ -151,7 +151,6 @@ int bochs_kms_init(struct bochs_device *bochs)
> > bochs->dev->mode_config.preferred_depth = 24;
> > bochs->dev->mode_config.prefer_shadow = 0;
> > bochs->dev->mode_config.prefer_shadow_fbdev = 1;
> > - bochs->dev->mode_config.fbdev_use_iomem = true;
> > bochs->dev->mode_config.quirk_addfb_prefer_host_byte_order = true;
> >
> > bochs->dev->mode_config.funcs = _mode_funcs;
> > diff --git a/drivers/gpu/drm/drm_fb_helper.c
> > b/drivers/gpu/drm/drm_fb_helper.c
> > index 343a292f2c7c..f345a314a437 100644
> > --- a/drivers/gpu/drm/drm_fb_helper.c
> > +++ b/drivers/gpu/drm/drm_fb_helper.c
> > @@ -388,24 +388,22 @@ static void drm_fb_helper_resume_worker(struct
> > work_struct *work)
> > }
> >
> > static void drm_fb_helper_dirty_blit_real(struct drm_fb_helper *fb_helper,
> > - struct drm_clip_rect *clip)
> > + struct drm_clip_rect *clip,
> > + struct dma_buf_map *dst)
> > {
> > struct drm_framebuffer *fb = fb_helper->fb;
> > unsigned int cpp = fb->format->cpp[0];
> > size_t offset = clip->y1 * fb->pitches[0] + clip->x1 * cpp;
> > void *src = fb_helper->fbdev->screen_buffer + offset;
> > - void *dst = fb_helper->buffer->map.vaddr + offset;
> > size_t len = (clip->x2 - clip->x1) * cpp;
> > unsigned int y;
> >
> > - for (y = clip->y1; y < clip->y2; y++) {
> > - if (!fb_helper->dev->mode_config.fbdev_use_iomem)
> > - memcpy(dst, src, len);
> > - else
> > - memcpy_toio((void __iomem *)dst, src, len);
> > + dma_buf_map_incr(dst, offset); /* go to first pixel within clip rect
> > */
> >
> > + for (y = clip->y1; y < clip->y2; y++) {
> > + dma_buf_map_memcpy_to(dst, src, len);
> > + dma_buf_map_incr(dst, fb->pitches[0]);
> > src += fb->pitches[0];
> > - dst += fb->pitches[0];
> > }
> > }
> >
> > @@ -433,8 +431,9 @@ static void drm_fb_helper_dirty_work(struct work_struct
> > *work)
> > ret = drm_client_buffer_vmap(helper->buffer, );
> > if (ret)
> > return;
> > - drm_fb_helper_dirty_blit_real(helper, _copy);
> > + drm_fb_helper_dirty_blit_real(helper, _copy,
> > );
> > }
> > +
> > if (helper->fb->funcs->dirty)
> > helper->fb->funcs->dirty(helper->fb, NULL, 0, 0,
> >_copy, 1);
> > @@ -771,6 +770,136 @@ void drm_fb_helper_sys_imageblit(struct fb_info *info,
> > }
> > EXPORT_SYMBOL(drm_fb_helper_sys_imageblit);
> >
Re: [Spice-devel] [PATCH v3 7/7] drm/todo: Update entries around struct dma_buf_map
On Tue, Sep 29, 2020 at 05:14:37PM +0200, Thomas Zimmermann wrote: > Instances of struct dma_buf_map should be useful throughout DRM's > memory management code. Furthermore, several drivers can now use > generic fbdev emulation. > > Signed-off-by: Thomas Zimmermann Acked-by: Daniel Vetter > --- > Documentation/gpu/todo.rst | 24 ++-- > 1 file changed, 22 insertions(+), 2 deletions(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 3751ac976c3e..023626c1837b 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -197,8 +197,10 @@ Convert drivers to use drm_fbdev_generic_setup() > > > Most drivers can use drm_fbdev_generic_setup(). Driver have to implement > -atomic modesetting and GEM vmap support. Current generic fbdev emulation > -expects the framebuffer in system memory (or system-like memory). > +atomic modesetting and GEM vmap support. Historically, generic fbdev > emulation > +expected the framebuffer in system memory or system-like memory. By employing > +struct dma_buf_map, drivers with frambuffers in I/O memory can be supported > +as well. > > Contact: Maintainer of the driver you plan to convert > > @@ -446,6 +448,24 @@ Contact: Ville Syrjälä, Daniel Vetter > > Level: Intermediate > > +Use struct dma_buf_map throughout codebase > +-- > + > +Pointers to shared device memory are stored in struct dma_buf_map. Each > +instance knows whether it refers to system or I/O memory. Most of the > DRM-wide > +interface have been converted to use struct dma_buf_map, but implementations > +often still use raw pointers. > + > +The task is to use struct dma_buf_map where it makes sense. > + > +* Memory managers should use struct dma_buf_map for dma-buf-imported buffers. > +* TTM might benefit from using struct dma_buf_map internally. > +* Framebuffer copying and blitting helpers should operate on struct > dma_buf_map. > + > +Contact: Thomas Zimmermann , Christian König, Daniel > Vetter > + > +Level: Intermediate > + > > Core refactorings > = > -- > 2.28.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 6/7] drm/fb_helper: Support framebuffers in I/O memory
memory.
> + * A set up instance of struct dma_buf_map can be used to access or
> manipulate
> + * the buffer memory. Depending on the location of the memory, the provided
> + * helpers will pick the correct operations. Data can be copied into the
> memory
> + * with dma_buf_map_memcpy_to(). The address can be manipulated with
> + * dma_buf_map_incr().
> *
> - * The type :c:type:`struct dma_buf_map ` and its helpers are
> - * actually independent from the dma-buf infrastructure. When sharing buffers
> - * among devices, drivers have to know the location of the memory to access
> - * the buffers in a safe way. :c:type:`struct dma_buf_map `
> - * solves this problem for dma-buf and its users. If other drivers or
> - * sub-systems require similar functionality, the type could be generalized
> - * and moved to a more prominent header file.
> + * .. code-block:: c
> + *
> + * const void *src = ...; // source buffer
> + * size_t len = ...; // length of src
> + *
> + * dma_buf_map_memcpy_to(, src, len);
> + * dma_buf_map_incr(, len); // go to first byte after the memcpy
> */
>
> /**
> @@ -210,4 +228,38 @@ static inline void dma_buf_map_clear(struct dma_buf_map
> *map)
> }
> }
>
> +/**
> + * dma_buf_map_memcpy_to - Memcpy into dma-buf mapping
> + * @dst: The dma-buf mapping structure
> + * @src: The source buffer
> + * @len: The number of byte in src
> + *
> + * Copies data into a dma-buf mapping. The source buffer is in system
> + * memory. Depending on the buffer's location, the helper picks the correct
> + * method of accessing the memory.
> + */
> +static inline void dma_buf_map_memcpy_to(struct dma_buf_map *dst, const void
> *src, size_t len)
> +{
> + if (dst->is_iomem)
> + memcpy_toio(dst->vaddr_iomem, src, len);
> + else
> + memcpy(dst->vaddr, src, len);
> +}
> +
> +/**
> + * dma_buf_map_incr - Increments the address stored in a dma-buf mapping
> + * @map: The dma-buf mapping structure
> + * @incr:The number of bytes to increment
> + *
> + * Increments the address stored in a dma-buf mapping. Depending on the
> + * buffer's location, the correct value will be updated.
> + */
> +static inline void dma_buf_map_incr(struct dma_buf_map *map, size_t incr)
> +{
> + if (map->is_iomem)
> + map->vaddr_iomem += incr;
> + else
> + map->vaddr += incr;
> +}
> +
> #endif /* __DMA_BUF_MAP_H__ */
> --
> 2.28.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 4/7] drm/gem: Update internal GEM vmap/vunmap interfaces to use struct dma_buf_map
bugfs_crc.c */
> #if defined(CONFIG_DEBUG_FS)
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index 89e2a2496734..cb8fbeeb731b 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -667,21 +667,15 @@ EXPORT_SYMBOL(drm_gem_unmap_dma_buf);
> *
> * Sets up a kernel virtual mapping. This can be used as the
> _buf_ops.vmap
> * callback. Calls into _gem_object_funcs.vmap for device specific
> handling.
> + * The kernel virtual address is returned in map.
> *
> - * Returns the kernel virtual address or NULL on failure.
> + * Returns 0 on success or a negative errno code otherwise.
> */
> int drm_gem_dmabuf_vmap(struct dma_buf *dma_buf, struct dma_buf_map *map)
> {
> struct drm_gem_object *obj = dma_buf->priv;
> - void *vaddr;
>
> - vaddr = drm_gem_vmap(obj);
> - if (IS_ERR(vaddr))
> - return PTR_ERR(vaddr);
> -
> - dma_buf_map_set_vaddr(map, vaddr);
> -
> - return 0;
> + return drm_gem_vmap(obj, map);
> }
> EXPORT_SYMBOL(drm_gem_dmabuf_vmap);
>
> @@ -697,7 +691,7 @@ void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf,
> struct dma_buf_map *map)
> {
> struct drm_gem_object *obj = dma_buf->priv;
>
> - drm_gem_vunmap(obj, map->vaddr);
> + drm_gem_vunmap(obj, map);
> }
> EXPORT_SYMBOL(drm_gem_dmabuf_vunmap);
Some of the transitional stuff disappearing!
Reviewed-by: Daniel Vetter
>
> --
> 2.28.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Fri, Oct 2, 2020 at 1:30 PM Christian König wrote: > > Am 02.10.20 um 11:58 schrieb Daniel Vetter: > > On Wed, Sep 30, 2020 at 02:51:46PM +0200, Daniel Vetter wrote: > >> On Wed, Sep 30, 2020 at 2:34 PM Christian König > >> wrote: > >>> Am 30.09.20 um 11:47 schrieb Daniel Vetter: > >>>> On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote: > >>>>> Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: > >>>>>> Hi > >>>>>> > >>>>>> Am 30.09.20 um 10:05 schrieb Christian König: > >>>>>>> Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: > >>>>>>>> Hi Christian > >>>>>>>> > >>>>>>>> Am 29.09.20 um 17:35 schrieb Christian König: > >>>>>>>>> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: > >>>>>>>>>> The new helper ttm_kmap_obj_to_dma_buf() extracts address and > >>>>>>>>>> location > >>>>>>>>>> from and instance of TTM's kmap_obj and initializes struct > >>>>>>>>>> dma_buf_map > >>>>>>>>>> with these values. Helpful for TTM-based drivers. > >>>>>>>>> We could completely drop that if we use the same structure inside > >>>>>>>>> TTM as > >>>>>>>>> well. > >>>>>>>>> > >>>>>>>>> Additional to that which driver is going to use this? > >>>>>>>> As Daniel mentioned, it's in patch 3. The TTM-based drivers will > >>>>>>>> retrieve the pointer via this function. > >>>>>>>> > >>>>>>>> I do want to see all that being more tightly integrated into TTM, but > >>>>>>>> not in this series. This one is about fixing the bochs-on-sparc64 > >>>>>>>> problem for good. Patch 7 adds an update to TTM to the DRM TODO list. > >>>>>>> I should have asked which driver you try to fix here :) > >>>>>>> > >>>>>>> In this case just keep the function inside bochs and only fix it > >>>>>>> there. > >>>>>>> > >>>>>>> All other drivers can be fixed when we generally pump this through > >>>>>>> TTM. > >>>>>> Did you take a look at patch 3? This function will be used by VRAM > >>>>>> helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we > >>>>>> have to duplicate the functionality in each if these drivers. Bochs > >>>>>> itself uses VRAM helpers and doesn't touch the function directly. > >>>>> Ah, ok can we have that then only in the VRAM helpers? > >>>>> > >>>>> Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj > >>>>> directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. > >>>>> > >>>>> What I want to avoid is to have another conversion function in TTM > >>>>> because > >>>>> what happens here is that we already convert from ttm_bus_placement to > >>>>> ttm_bo_kmap_obj and then to dma_buf_map. > >>>> Hm I'm not really seeing how that helps with a gradual conversion of > >>>> everything over to dma_buf_map and assorted helpers for access? There's > >>>> too many places in ttm drivers where is_iomem and related stuff is used > >>>> to > >>>> be able to convert it all in one go. An intermediate state with a bunch > >>>> of > >>>> conversions seems fairly unavoidable to me. > >>> Fair enough. I would just have started bottom up and not top down. > >>> > >>> Anyway feel free to go ahead with this approach as long as we can remove > >>> the new function again when we clean that stuff up for good. > >> Yeah I guess bottom up would make more sense as a refactoring. But the > >> main motivation to land this here is to fix the __mmio vs normal > >> memory confusion in the fbdev emulation helpers for sparc (and > >> anything else that needs this). Hence the top down approach for > >> rolling this out. > > Ok I started reviewing this a bit more in-depth, and I think this is a bit > > too much of a de-tour. > > > > Looking through al
Re: [Spice-devel] [PATCH v3 3/7] drm/gem: Use struct dma_buf_map in GEM vmap ops and convert GEM backends
On Tue, Sep 29, 2020 at 05:14:33PM +0200, Thomas Zimmermann wrote:
> This patch replaces the vmap/vunmap's use of raw pointers in GEM object
> functions with instances of struct dma_buf_map. GEM backends are
> converted as well.
>
> For most GEM backends, this simply change the returned type. GEM VRAM
> helpers are also updated to indicate whether the returned framebuffer
> address is in system or I/O memory.
>
> Signed-off-by: Thomas Zimmermann
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 14 ++--
> drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h | 4 +-
> drivers/gpu/drm/ast/ast_cursor.c| 29 +++
> drivers/gpu/drm/ast/ast_drv.h | 7 +-
> drivers/gpu/drm/drm_gem.c | 22 ++---
> drivers/gpu/drm/drm_gem_cma_helper.c| 14 ++--
> drivers/gpu/drm/drm_gem_shmem_helper.c | 48 ++-
> drivers/gpu/drm/drm_gem_vram_helper.c | 90 +++--
> drivers/gpu/drm/etnaviv/etnaviv_drv.h | 4 +-
> drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 11 ++-
> drivers/gpu/drm/exynos/exynos_drm_gem.c | 6 +-
> drivers/gpu/drm/exynos/exynos_drm_gem.h | 4 +-
> drivers/gpu/drm/lima/lima_gem.c | 6 +-
> drivers/gpu/drm/lima/lima_sched.c | 11 ++-
> drivers/gpu/drm/mgag200/mgag200_mode.c | 12 +--
> drivers/gpu/drm/nouveau/nouveau_gem.h | 4 +-
> drivers/gpu/drm/nouveau/nouveau_prime.c | 9 ++-
> drivers/gpu/drm/panfrost/panfrost_perfcnt.c | 14 ++--
> drivers/gpu/drm/qxl/qxl_display.c | 13 +--
> drivers/gpu/drm/qxl/qxl_draw.c | 16 ++--
> drivers/gpu/drm/qxl/qxl_drv.h | 8 +-
> drivers/gpu/drm/qxl/qxl_object.c| 23 +++---
> drivers/gpu/drm/qxl/qxl_object.h| 2 +-
> drivers/gpu/drm/qxl/qxl_prime.c | 12 +--
> drivers/gpu/drm/radeon/radeon_gem.c | 4 +-
> drivers/gpu/drm/radeon/radeon_prime.c | 9 ++-
> drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 22 +++--
> drivers/gpu/drm/rockchip/rockchip_drm_gem.h | 4 +-
> drivers/gpu/drm/tiny/cirrus.c | 10 ++-
> drivers/gpu/drm/tiny/gm12u320.c | 10 ++-
> drivers/gpu/drm/udl/udl_modeset.c | 8 +-
> drivers/gpu/drm/vboxvideo/vbox_mode.c | 11 ++-
> drivers/gpu/drm/vc4/vc4_bo.c| 6 +-
> drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
> drivers/gpu/drm/vgem/vgem_drv.c | 16 ++--
> drivers/gpu/drm/xen/xen_drm_front_gem.c | 18 +++--
> drivers/gpu/drm/xen/xen_drm_front_gem.h | 6 +-
> include/drm/drm_gem.h | 5 +-
> include/drm/drm_gem_cma_helper.h| 4 +-
> include/drm/drm_gem_shmem_helper.h | 4 +-
> include/drm/drm_gem_vram_helper.h | 4 +-
> 41 files changed, 304 insertions(+), 222 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index 5b465ab774d1..de7d0cfe1b93 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> @@ -44,13 +44,14 @@
> /**
> * amdgpu_gem_prime_vmap - _buf_ops.vmap implementation
> * @obj: GEM BO
> + * @map: The virtual address of the mapping.
> *
> * Sets up an in-kernel virtual mapping of the BO's memory.
> *
> * Returns:
> - * The virtual address of the mapping or an error pointer.
> + * 0 on success, or a negative errno code otherwise.
> */
> -void *amdgpu_gem_prime_vmap(struct drm_gem_object *obj)
> +int amdgpu_gem_prime_vmap(struct drm_gem_object *obj, struct dma_buf_map
> *map)
> {
> struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
> int ret;
> @@ -58,19 +59,20 @@ void *amdgpu_gem_prime_vmap(struct drm_gem_object *obj)
> ret = ttm_bo_kmap(>tbo, 0, bo->tbo.num_pages,
> >dma_buf_vmap);
> if (ret)
> - return ERR_PTR(ret);
> + return ret;
> + ttm_kmap_obj_to_dma_buf_map(>dma_buf_vmap, map);
I guess with the ttm_bo_vmap idea all the ttm changes here will look a bit
different.
>
> - return bo->dma_buf_vmap.virtual;
> + return 0;
> }
>
> /**
> * amdgpu_gem_prime_vunmap - _buf_ops.vunmap implementation
> * @obj: GEM BO
> - * @vaddr: Virtual address (unused)
> + * @map: Virtual address (unused)
> *
> * Tears down the in-kernel virtual mapping of the BO's memory.
> */
> -void amdgpu_gem_prime_vunmap(struct drm_gem_object *obj, void *vaddr)
> +void amdgpu_gem_prime_vunmap(struct drm_gem_object *obj, struct dma_buf_map
> *map)
> {
> struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
> index 2c5c84a06bb9..622642793064 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
> @@ -31,8 +31,8 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct
> drm_device *dev,
>
Re: [Spice-devel] [PATCH v3 5/7] drm/gem: Store client buffer mappings as struct dma_buf_map
> struct drm_clip_rect *clip = >dirty_clip;
> struct drm_clip_rect clip_copy;
> unsigned long flags;
> - void *vaddr;
> + struct dma_buf_map map;
> + int ret;
>
> spin_lock_irqsave(>dirty_lock, flags);
> clip_copy = *clip;
> @@ -429,8 +430,8 @@ static void drm_fb_helper_dirty_work(struct work_struct
> *work)
>
> /* Generic fbdev uses a shadow buffer */
> if (helper->buffer) {
> - vaddr = drm_client_buffer_vmap(helper->buffer);
> - if (IS_ERR(vaddr))
> + ret = drm_client_buffer_vmap(helper->buffer, );
> + if (ret)
> return;
> drm_fb_helper_dirty_blit_real(helper, _copy);
> }
> @@ -2076,7 +2077,8 @@ static int drm_fb_helper_generic_probe(struct
> drm_fb_helper *fb_helper,
> struct drm_framebuffer *fb;
> struct fb_info *fbi;
> u32 format;
> - void *vaddr;
> + struct dma_buf_map map;
> + int ret;
>
> drm_dbg_kms(dev, "surface width(%d), height(%d) and bpp(%d)\n",
> sizes->surface_width, sizes->surface_height,
> @@ -2112,11 +2114,14 @@ static int drm_fb_helper_generic_probe(struct
> drm_fb_helper *fb_helper,
> fb_deferred_io_init(fbi);
> } else {
> /* buffer is mapped for HW framebuffer */
> - vaddr = drm_client_buffer_vmap(fb_helper->buffer);
> - if (IS_ERR(vaddr))
> - return PTR_ERR(vaddr);
> + ret = drm_client_buffer_vmap(fb_helper->buffer, );
> + if (ret)
> + return ret;
> + if (map.is_iomem)
> + fbi->screen_base = map.vaddr_iomem;
> + else
> + fbi->screen_buffer = map.vaddr;
>
> - fbi->screen_buffer = vaddr;
> /* Shamelessly leak the physical address to user-space */
> #if IS_ENABLED(CONFIG_DRM_FBDEV_LEAK_PHYS_SMEM)
> if (drm_leak_fbdev_smem && fbi->fix.smem_start == 0)
> diff --git a/include/drm/drm_client.h b/include/drm/drm_client.h
> index 7aaea665bfc2..f07f2fb02e75 100644
> --- a/include/drm/drm_client.h
> +++ b/include/drm/drm_client.h
> @@ -3,6 +3,7 @@
> #ifndef _DRM_CLIENT_H_
> #define _DRM_CLIENT_H_
>
> +#include
> #include
> #include
> #include
> @@ -141,9 +142,9 @@ struct drm_client_buffer {
> struct drm_gem_object *gem;
>
> /**
> - * @vaddr: Virtual address for the buffer
> + * @map: Virtual address for the buffer
>*/
> - void *vaddr;
> + struct dma_buf_map map;
>
> /**
>* @fb: DRM framebuffer
> @@ -155,7 +156,7 @@ struct drm_client_buffer *
> drm_client_framebuffer_create(struct drm_client_dev *client, u32 width, u32
> height, u32 format);
> void drm_client_framebuffer_delete(struct drm_client_buffer *buffer);
> int drm_client_framebuffer_flush(struct drm_client_buffer *buffer, struct
> drm_rect *rect);
> -void *drm_client_buffer_vmap(struct drm_client_buffer *buffer);
> +int drm_client_buffer_vmap(struct drm_client_buffer *buffer, struct
> dma_buf_map *map);
> void drm_client_buffer_vunmap(struct drm_client_buffer *buffer);
>
> int drm_client_modeset_create(struct drm_client_dev *client);
Reviewed-by: Daniel Vetter
> --
> 2.28.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Wed, Sep 30, 2020 at 02:51:46PM +0200, Daniel Vetter wrote: > On Wed, Sep 30, 2020 at 2:34 PM Christian König > wrote: > > > > Am 30.09.20 um 11:47 schrieb Daniel Vetter: > > > On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote: > > >> Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: > > >>> Hi > > >>> > > >>> Am 30.09.20 um 10:05 schrieb Christian König: > > >>>> Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: > > >>>>> Hi Christian > > >>>>> > > >>>>> Am 29.09.20 um 17:35 schrieb Christian König: > > >>>>>> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: > > >>>>>>> The new helper ttm_kmap_obj_to_dma_buf() extracts address and > > >>>>>>> location > > >>>>>>> from and instance of TTM's kmap_obj and initializes struct > > >>>>>>> dma_buf_map > > >>>>>>> with these values. Helpful for TTM-based drivers. > > >>>>>> We could completely drop that if we use the same structure inside > > >>>>>> TTM as > > >>>>>> well. > > >>>>>> > > >>>>>> Additional to that which driver is going to use this? > > >>>>> As Daniel mentioned, it's in patch 3. The TTM-based drivers will > > >>>>> retrieve the pointer via this function. > > >>>>> > > >>>>> I do want to see all that being more tightly integrated into TTM, but > > >>>>> not in this series. This one is about fixing the bochs-on-sparc64 > > >>>>> problem for good. Patch 7 adds an update to TTM to the DRM TODO list. > > >>>> I should have asked which driver you try to fix here :) > > >>>> > > >>>> In this case just keep the function inside bochs and only fix it there. > > >>>> > > >>>> All other drivers can be fixed when we generally pump this through TTM. > > >>> Did you take a look at patch 3? This function will be used by VRAM > > >>> helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we > > >>> have to duplicate the functionality in each if these drivers. Bochs > > >>> itself uses VRAM helpers and doesn't touch the function directly. > > >> Ah, ok can we have that then only in the VRAM helpers? > > >> > > >> Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj > > >> directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. > > >> > > >> What I want to avoid is to have another conversion function in TTM > > >> because > > >> what happens here is that we already convert from ttm_bus_placement to > > >> ttm_bo_kmap_obj and then to dma_buf_map. > > > Hm I'm not really seeing how that helps with a gradual conversion of > > > everything over to dma_buf_map and assorted helpers for access? There's > > > too many places in ttm drivers where is_iomem and related stuff is used to > > > be able to convert it all in one go. An intermediate state with a bunch of > > > conversions seems fairly unavoidable to me. > > > > Fair enough. I would just have started bottom up and not top down. > > > > Anyway feel free to go ahead with this approach as long as we can remove > > the new function again when we clean that stuff up for good. > > Yeah I guess bottom up would make more sense as a refactoring. But the > main motivation to land this here is to fix the __mmio vs normal > memory confusion in the fbdev emulation helpers for sparc (and > anything else that needs this). Hence the top down approach for > rolling this out. Ok I started reviewing this a bit more in-depth, and I think this is a bit too much of a de-tour. Looking through all the callers of ttm_bo_kmap almost everyone maps the entire object. Only vmwgfx uses to map less than that. Also, everyone just immediately follows up with converting that full object map into a pointer. So I think what we really want here is: - new function int ttm_bo_vmap(struct ttm_buffer_object *bo, struct dma_buf_map *map); _vmap name since that's consistent with both dma_buf functions and what's usually used to implement this. Outside of the ttm world kmap usually just means single-page mappings using kmap() or it's iomem sibling io_mapping_map* so rather confusing name for a function which usually is just used to set up a vmap of the entire buffer. - a h
Re: [Spice-devel] [PATCH v3 1/7] drm/vram-helper: Remove invariant parameters from internal kmap function
On Tue, Sep 29, 2020 at 05:14:31PM +0200, Thomas Zimmermann wrote:
> The parameters map and is_iomem are always of the same value. Removed them
> to prepares the function for conversion to struct dma_buf_map.
>
> Signed-off-by: Thomas Zimmermann
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/drm_gem_vram_helper.c | 17 ++---
> 1 file changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c
> b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 3fe4b326e18e..256b346664f2 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -382,16 +382,16 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
> }
> EXPORT_SYMBOL(drm_gem_vram_unpin);
>
> -static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo,
> - bool map, bool *is_iomem)
> +static void *drm_gem_vram_kmap_locked(struct drm_gem_vram_object *gbo)
> {
> int ret;
> struct ttm_bo_kmap_obj *kmap = >kmap;
> + bool is_iomem;
>
> if (gbo->kmap_use_count > 0)
> goto out;
>
> - if (kmap->virtual || !map)
> + if (kmap->virtual)
> goto out;
>
> ret = ttm_bo_kmap(>bo, 0, gbo->bo.num_pages, kmap);
> @@ -399,15 +399,10 @@ static void *drm_gem_vram_kmap_locked(struct
> drm_gem_vram_object *gbo,
> return ERR_PTR(ret);
>
> out:
> - if (!kmap->virtual) {
> - if (is_iomem)
> - *is_iomem = false;
> + if (!kmap->virtual)
> return NULL; /* not mapped; don't increment ref */
> - }
> ++gbo->kmap_use_count;
> - if (is_iomem)
> - return ttm_kmap_obj_virtual(kmap, is_iomem);
> - return kmap->virtual;
> + return ttm_kmap_obj_virtual(kmap, _iomem);
> }
>
> static void drm_gem_vram_kunmap_locked(struct drm_gem_vram_object *gbo)
> @@ -452,7 +447,7 @@ void *drm_gem_vram_vmap(struct drm_gem_vram_object *gbo)
> ret = drm_gem_vram_pin_locked(gbo, 0);
> if (ret)
> goto err_ttm_bo_unreserve;
> - base = drm_gem_vram_kmap_locked(gbo, true, NULL);
> + base = drm_gem_vram_kmap_locked(gbo);
> if (IS_ERR(base)) {
> ret = PTR_ERR(base);
> goto err_drm_gem_vram_unpin_locked;
> --
> 2.28.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Wed, Sep 30, 2020 at 2:34 PM Christian König wrote: > > Am 30.09.20 um 11:47 schrieb Daniel Vetter: > > On Wed, Sep 30, 2020 at 10:34:31AM +0200, Christian König wrote: > >> Am 30.09.20 um 10:19 schrieb Thomas Zimmermann: > >>> Hi > >>> > >>> Am 30.09.20 um 10:05 schrieb Christian König: > >>>> Am 29.09.20 um 19:49 schrieb Thomas Zimmermann: > >>>>> Hi Christian > >>>>> > >>>>> Am 29.09.20 um 17:35 schrieb Christian König: > >>>>>> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann: > >>>>>>> The new helper ttm_kmap_obj_to_dma_buf() extracts address and location > >>>>>>> from and instance of TTM's kmap_obj and initializes struct dma_buf_map > >>>>>>> with these values. Helpful for TTM-based drivers. > >>>>>> We could completely drop that if we use the same structure inside TTM > >>>>>> as > >>>>>> well. > >>>>>> > >>>>>> Additional to that which driver is going to use this? > >>>>> As Daniel mentioned, it's in patch 3. The TTM-based drivers will > >>>>> retrieve the pointer via this function. > >>>>> > >>>>> I do want to see all that being more tightly integrated into TTM, but > >>>>> not in this series. This one is about fixing the bochs-on-sparc64 > >>>>> problem for good. Patch 7 adds an update to TTM to the DRM TODO list. > >>>> I should have asked which driver you try to fix here :) > >>>> > >>>> In this case just keep the function inside bochs and only fix it there. > >>>> > >>>> All other drivers can be fixed when we generally pump this through TTM. > >>> Did you take a look at patch 3? This function will be used by VRAM > >>> helpers, nouveau, radeon, amdgpu and qxl. If we don't put it here, we > >>> have to duplicate the functionality in each if these drivers. Bochs > >>> itself uses VRAM helpers and doesn't touch the function directly. > >> Ah, ok can we have that then only in the VRAM helpers? > >> > >> Alternative you could go ahead and use dma_buf_map in ttm_bo_kmap_obj > >> directly and drop the hack with the TTM_BO_MAP_IOMEM_MASK. > >> > >> What I want to avoid is to have another conversion function in TTM because > >> what happens here is that we already convert from ttm_bus_placement to > >> ttm_bo_kmap_obj and then to dma_buf_map. > > Hm I'm not really seeing how that helps with a gradual conversion of > > everything over to dma_buf_map and assorted helpers for access? There's > > too many places in ttm drivers where is_iomem and related stuff is used to > > be able to convert it all in one go. An intermediate state with a bunch of > > conversions seems fairly unavoidable to me. > > Fair enough. I would just have started bottom up and not top down. > > Anyway feel free to go ahead with this approach as long as we can remove > the new function again when we clean that stuff up for good. Yeah I guess bottom up would make more sense as a refactoring. But the main motivation to land this here is to fix the __mmio vs normal memory confusion in the fbdev emulation helpers for sparc (and anything else that needs this). Hence the top down approach for rolling this out. -Daniel > > Christian. > > > -Daniel > > > >> Thanks, > >> Christian. > >> > >>> Best regards > >>> Thomas > >>> > >>>> Regards, > >>>> Christian. > >>>> > >>>>> Best regards > >>>>> Thomas > >>>>> > >>>>>> Regards, > >>>>>> Christian. > >>>>>> > >>>>>>> Signed-off-by: Thomas Zimmermann > >>>>>>> --- > >>>>>>> include/drm/ttm/ttm_bo_api.h | 24 > >>>>>>> include/linux/dma-buf-map.h | 20 > >>>>>>> 2 files changed, 44 insertions(+) > >>>>>>> > >>>>>>> diff --git a/include/drm/ttm/ttm_bo_api.h > >>>>>>> b/include/drm/ttm/ttm_bo_api.h > >>>>>>> index c96a25d571c8..62d89f05a801 100644 > >>>>>>> --- a/include/drm/ttm/ttm_bo_api.h > >>>>>>> +++ b/include/drm/ttm/ttm_bo_api.h > >>>>>>>
Re: [Spice-devel] [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
; > > > > + * is not mapped, the returned mapping is initialized to NULL.
> > > > > > + */
> > > > > > +static inline void ttm_kmap_obj_to_dma_buf_map(struct
> > > > > > ttm_bo_kmap_obj
> > > > > > *kmap,
> > > > > > + struct dma_buf_map *map)
> > > > > > +{
> > > > > > + bool is_iomem;
> > > > > > + void *vaddr = ttm_kmap_obj_virtual(kmap, _iomem);
> > > > > > +
> > > > > > + if (!vaddr)
> > > > > > + dma_buf_map_clear(map);
> > > > > > + else if (is_iomem)
> > > > > > + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem
> > > > > > *)vaddr);
> > > > > > + else
> > > > > > + dma_buf_map_set_vaddr(map, vaddr);
> > > > > > +}
> > > > > > +
> > > > > > /**
> > > > > > * ttm_bo_kmap
> > > > > > *
> > > > > > diff --git a/include/linux/dma-buf-map.h
> > > > > > b/include/linux/dma-buf-map.h
> > > > > > index fd1aba545fdf..2e8bbecb5091 100644
> > > > > > --- a/include/linux/dma-buf-map.h
> > > > > > +++ b/include/linux/dma-buf-map.h
> > > > > > @@ -45,6 +45,12 @@
> > > > > > *
> > > > > > * dma_buf_map_set_vaddr( 0xdeadbeaf);
> > > > > > *
> > > > > > + * To set an address in I/O memory, use
> > > > > > dma_buf_map_set_vaddr_iomem().
> > > > > > + *
> > > > > > + * .. code-block:: c
> > > > > > + *
> > > > > > + * dma_buf_map_set_vaddr_iomem( 0xdeadbeaf);
> > > > > > + *
> > > > > > * Test if a mapping is valid with either dma_buf_map_is_set() or
> > > > > > * dma_buf_map_is_null().
> > > > > > *
> > > > > > @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct
> > > > > > dma_buf_map *map, void *vaddr)
> > > > > > map->is_iomem = false;
> > > > > > }
> > > > > > +/**
> > > > > > + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure
> > > > > > to
> > > > > > an address in I/O memory
> > > > > > + * @map: The dma-buf mapping structure
> > > > > > + * @vaddr_iomem: An I/O-memory address
> > > > > > + *
> > > > > > + * Sets the address and the I/O-memory flag.
> > > > > > + */
> > > > > > +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map
> > > > > > *map,
> > > > > > + void __iomem *vaddr_iomem)
> > > > > > +{
> > > > > > + map->vaddr_iomem = vaddr_iomem;
> > > > > > + map->is_iomem = true;
> > > > > > +}
> > > > > > +
> > > > > > /**
> > > > > > * dma_buf_map_is_equal - Compares two dma-buf mapping structures
> > > > > > for equality
> > > > > > * @lhs: The dma-buf mapping structure
> > > > > ___
> > > > > dri-devel mailing list
> > > > > dri-de...@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > > > ___
> > > > amd-gfx mailing list
> > > > amd-...@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> > >
> > > ___
> > > dri-devel mailing list
> > > dri-de...@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > >
> >
> > ___
> > amd-gfx mailing list
> > amd-...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 2/7] drm/ttm: Add ttm_kmap_obj_to_dma_buf_map() for type conversion
On Tue, Sep 29, 2020 at 5:35 PM Christian König
wrote:
>
> Am 29.09.20 um 17:14 schrieb Thomas Zimmermann:
> > The new helper ttm_kmap_obj_to_dma_buf() extracts address and location
> > from and instance of TTM's kmap_obj and initializes struct dma_buf_map
> > with these values. Helpful for TTM-based drivers.
>
> We could completely drop that if we use the same structure inside TTM as
> well.
> Additional to that which driver is going to use this?
Patch 3 in this series.
I also think this makes sense for gradual conversion:
1. add this helper
2. convert over all users of vmap, this should get rid of is_iomem
flags (and will probably result in a pile of small additions to
dma-buf-map.h)
3. push the struct dma_buf_map down into ttm drivers
Cheers, Daniel
> Regards,
> Christian.
>
> >
> > Signed-off-by: Thomas Zimmermann
> > ---
> > include/drm/ttm/ttm_bo_api.h | 24
> > include/linux/dma-buf-map.h | 20
> > 2 files changed, 44 insertions(+)
> >
> > diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> > index c96a25d571c8..62d89f05a801 100644
> > --- a/include/drm/ttm/ttm_bo_api.h
> > +++ b/include/drm/ttm/ttm_bo_api.h
> > @@ -34,6 +34,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -486,6 +487,29 @@ static inline void *ttm_kmap_obj_virtual(struct
> > ttm_bo_kmap_obj *map,
> > return map->virtual;
> > }
> >
> > +/**
> > + * ttm_kmap_obj_to_dma_buf_map
> > + *
> > + * @kmap: A struct ttm_bo_kmap_obj returned from ttm_bo_kmap.
> > + * @map: Returns the mapping as struct dma_buf_map
> > + *
> > + * Converts struct ttm_bo_kmap_obj to struct dma_buf_map. If the memory
> > + * is not mapped, the returned mapping is initialized to NULL.
> > + */
> > +static inline void ttm_kmap_obj_to_dma_buf_map(struct ttm_bo_kmap_obj
> > *kmap,
> > +struct dma_buf_map *map)
> > +{
> > + bool is_iomem;
> > + void *vaddr = ttm_kmap_obj_virtual(kmap, _iomem);
> > +
> > + if (!vaddr)
> > + dma_buf_map_clear(map);
> > + else if (is_iomem)
> > + dma_buf_map_set_vaddr_iomem(map, (void __force __iomem
> > *)vaddr);
> > + else
> > + dma_buf_map_set_vaddr(map, vaddr);
> > +}
> > +
> > /**
> >* ttm_bo_kmap
> >*
> > diff --git a/include/linux/dma-buf-map.h b/include/linux/dma-buf-map.h
> > index fd1aba545fdf..2e8bbecb5091 100644
> > --- a/include/linux/dma-buf-map.h
> > +++ b/include/linux/dma-buf-map.h
> > @@ -45,6 +45,12 @@
> >*
> >* dma_buf_map_set_vaddr( 0xdeadbeaf);
> >*
> > + * To set an address in I/O memory, use dma_buf_map_set_vaddr_iomem().
> > + *
> > + * .. code-block:: c
> > + *
> > + * dma_buf_map_set_vaddr_iomem( 0xdeadbeaf);
> > + *
> >* Test if a mapping is valid with either dma_buf_map_is_set() or
> >* dma_buf_map_is_null().
> >*
> > @@ -118,6 +124,20 @@ static inline void dma_buf_map_set_vaddr(struct
> > dma_buf_map *map, void *vaddr)
> > map->is_iomem = false;
> > }
> >
> > +/**
> > + * dma_buf_map_set_vaddr_iomem - Sets a dma-buf mapping structure to an
> > address in I/O memory
> > + * @map: The dma-buf mapping structure
> > + * @vaddr_iomem: An I/O-memory address
> > + *
> > + * Sets the address and the I/O-memory flag.
> > + */
> > +static inline void dma_buf_map_set_vaddr_iomem(struct dma_buf_map *map,
> > +void __iomem *vaddr_iomem)
> > +{
> > + map->vaddr_iomem = vaddr_iomem;
> > + map->is_iomem = true;
> > +}
> > +
> > /**
> >* dma_buf_map_is_equal - Compares two dma-buf mapping structures for
> > equality
> >* @lhs:The dma-buf mapping structure
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 4/4] drm/qxl: use qxl pin function
On Tue, Sep 29, 2020 at 11:51:15AM +0200, Gerd Hoffmann wrote:
> Otherwise ttm throws a WARN because we try to pin without a reservation.
>
> Fixes: 9d36d4320462 ("drm/qxl: switch over to the new pin interface")
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/qxl/qxl_object.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_object.c
> b/drivers/gpu/drm/qxl/qxl_object.c
> index d3635e3e3267..eb45267d51db 100644
> --- a/drivers/gpu/drm/qxl/qxl_object.c
> +++ b/drivers/gpu/drm/qxl/qxl_object.c
> @@ -145,7 +145,7 @@ int qxl_bo_create(struct qxl_device *qdev,
> return r;
> }
> if (pinned)
> - ttm_bo_pin(>tbo);
> + qxl_bo_pin(bo);
I think this is now after ttm_bo_init, and at that point the object is
visible to lru users and everything. So I do think you need to grab locks
here instead of just incrementing the pin count alone.
It's also I think a bit racy, since ttm_bo_init drops the lock, so someone
might have snuck in and evicted the object already.
I think what you need is to call ttm_bo_init_reserved, then ttm_bo_pin,
then ttm_bo_unreserve, all explicitly.
-Daniel
> *bo_ptr = bo;
> return 0;
> }
> --
> 2.27.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] drm/qxl: use drmm_mode_config_init
On Tue, Sep 08, 2020 at 11:39:10AM +0200, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
Btw going all in on devm_drm_dev_alloc and managed functions might be good
cleanup for virtio.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/qxl/qxl_display.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> b/drivers/gpu/drm/qxl/qxl_display.c
> index fa79688013b7..4be04eaf7f37 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -1190,7 +1190,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
> int i;
> int ret;
>
> - drm_mode_config_init(>ddev);
> + ret = drmm_mode_config_init(>ddev);
> + if (ret)
> + return ret;
>
> ret = qxl_create_monitors_object(qdev);
> if (ret)
> @@ -1223,5 +1225,4 @@ int qxl_modeset_init(struct qxl_device *qdev)
> void qxl_modeset_fini(struct qxl_device *qdev)
> {
> qxl_destroy_monitors_object(qdev);
> - drm_mode_config_cleanup(>ddev);
> }
> --
> 2.27.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2] drm/qxl: don't take vga ports on rev5+
On Fri, Aug 07, 2020 at 12:55:01PM +0200, Gerd Hoffmann wrote:
> qemu 5.0 introduces a new qxl hardware revision 5. Unlike revision 4
> (and below) the device doesn't switch back into vga compatibility mode
> when someone touches the vga ports. So we don't have to reserve the
> vga ports any more to avoid that happening.
>
> Signed-off-by: Gerd Hoffmann
Does what it says on the label.
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 13872b882775..6e7f16f4cec7 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -96,7 +96,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> if (ret)
> goto disable_pci;
>
> - if (is_vga(pdev)) {
> + if (is_vga(pdev) && pdev->revision < 5) {
> ret = vga_get_interruptible(pdev, VGA_RSRC_LEGACY_IO);
> if (ret) {
> DRM_ERROR("can't get legacy vga ioports\n");
> @@ -127,7 +127,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> unload:
> qxl_device_fini(qdev);
> put_vga:
> - if (is_vga(pdev))
> + if (is_vga(pdev) && pdev->revision < 5)
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> disable_pci:
> pci_disable_device(pdev);
> @@ -155,7 +155,7 @@ qxl_pci_remove(struct pci_dev *pdev)
>
> drm_dev_unregister(dev);
> drm_atomic_helper_shutdown(dev);
> - if (is_vga(pdev))
> + if (is_vga(pdev) && pdev->revision < 5)
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> }
>
> --
> 2.18.4
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc
On Tue, Apr 28, 2020 at 07:00:26PM +0200, Sam Ravnborg wrote:
> On Tue, Apr 28, 2020 at 04:00:11PM +0200, Daniel Vetter wrote:
> > On Fri, Apr 24, 2020 at 05:09:11PM +0200, Sam Ravnborg wrote:
> > > Hi Daniel
> > >
> > > On Wed, Apr 15, 2020 at 09:40:01AM +0200, Daniel Vetter wrote:
> > > > Also need to remove the drm_dev_put from the remove hook.
> > > >
> > > > Acked-by: Gerd Hoffmann
> > > > Signed-off-by: Daniel Vetter
> > > > Cc: Dave Airlie
> > > > Cc: Gerd Hoffmann
> > > > Cc: virtualizat...@lists.linux-foundation.org
> > > > Cc: spice-devel@lists.freedesktop.org
> > > > ---
> > > > drivers/gpu/drm/qxl/qxl_drv.c | 15 ---
> > > > drivers/gpu/drm/qxl/qxl_drv.h | 3 +--
> > > > drivers/gpu/drm/qxl/qxl_kms.c | 12 +---
> > > > 3 files changed, 10 insertions(+), 20 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c
> > > > b/drivers/gpu/drm/qxl/qxl_drv.c
> > > > index 09102e2efabc..6b4ae4c5fb76 100644
> > > > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > > > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > > > @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > > > pci_device_id *ent)
> > > > return -EINVAL; /* TODO: ENODEV ? */
> > > > }
> > > >
> > > > - qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> > > > - if (!qdev)
> > > > + qdev = devm_drm_dev_alloc(>dev, _driver,
> > > > + struct qxl_device, ddev);
> > > > + if (IS_ERR(qdev)) {
> > > > + pr_err("Unable to init drm dev");
> > > > return -ENOMEM;
> > > > + }
> > >
> > > The other patches do not add any error message when devm_drm_dev_alloc()
> > > fails and driver core will log that driver init failed.
> > >
> > > So the pr_err() above should be dropped.
> > > I know it comes from qxl_device_init() but that does not make it a good
> > > idea.
> >
> > Hm I know we're inconsistent here, but some drivers have error logging on
> > all branches, some dont. I'm just trying to go with the prevailing style.
> >
> > > With this fixed:
> >
> > Insisting on this or ok as-is?
> OK as-is.
Ok, merged the two qxl patches too, plus a lot of the driver conversions.
Thanks a lot to everyone who commented, reviewed and tested thus far.
-Daniel
>
> Sam
>
> > -Daniel
> >
> > > Acked-by: Sam Ravnborg
> > >
> > > >
> > > > ret = pci_enable_device(pdev);
> > > > if (ret)
> > > > - goto free_dev;
> > > > + return ret;
> > > >
> > > > ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev,
> > > > "qxl");
> > > > if (ret)
> > > > @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > > > pci_device_id *ent)
> > > > }
> > > > }
> > > >
> > > > - ret = qxl_device_init(qdev, _driver, pdev);
> > > > + ret = qxl_device_init(qdev, pdev);
> > > > if (ret)
> > > > goto put_vga;
> > > >
> > > > @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > > > pci_device_id *ent)
> > > > vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > > > disable_pci:
> > > > pci_disable_device(pdev);
> > > > -free_dev:
> > > > - kfree(qdev);
> > > > +
> > > > return ret;
> > > > }
> > > >
> > > > @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
> > > > drm_atomic_helper_shutdown(dev);
> > > > if (is_vga(pdev))
> > > > vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > > > - drm_dev_put(dev);
> > > > }
> > > >
> > > > DEFINE_DRM_GEM_FOPS(qxl_fops);
> > > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.h
> > > > b/drivers/gpu/drm/qxl/qxl_drv.h
> > > > index 435126facc9b..86ac191d9205 100644
> > > > --- a/drivers/gpu/drm/qxl/qxl_drv.h
> > > > +++ b/driver
Re: [Spice-devel] [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc
On Fri, Apr 24, 2020 at 05:09:11PM +0200, Sam Ravnborg wrote:
> Hi Daniel
>
> On Wed, Apr 15, 2020 at 09:40:01AM +0200, Daniel Vetter wrote:
> > Also need to remove the drm_dev_put from the remove hook.
> >
> > Acked-by: Gerd Hoffmann
> > Signed-off-by: Daniel Vetter
> > Cc: Dave Airlie
> > Cc: Gerd Hoffmann
> > Cc: virtualizat...@lists.linux-foundation.org
> > Cc: spice-devel@lists.freedesktop.org
> > ---
> > drivers/gpu/drm/qxl/qxl_drv.c | 15 ---
> > drivers/gpu/drm/qxl/qxl_drv.h | 3 +--
> > drivers/gpu/drm/qxl/qxl_kms.c | 12 +---
> > 3 files changed, 10 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 09102e2efabc..6b4ae4c5fb76 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > return -EINVAL; /* TODO: ENODEV ? */
> > }
> >
> > - qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> > - if (!qdev)
> > + qdev = devm_drm_dev_alloc(>dev, _driver,
> > + struct qxl_device, ddev);
> > + if (IS_ERR(qdev)) {
> > + pr_err("Unable to init drm dev");
> > return -ENOMEM;
> > + }
>
> The other patches do not add any error message when devm_drm_dev_alloc()
> fails and driver core will log that driver init failed.
>
> So the pr_err() above should be dropped.
> I know it comes from qxl_device_init() but that does not make it a good
> idea.
Hm I know we're inconsistent here, but some drivers have error logging on
all branches, some dont. I'm just trying to go with the prevailing style.
> With this fixed:
Insisting on this or ok as-is?
-Daniel
> Acked-by: Sam Ravnborg
>
> >
> > ret = pci_enable_device(pdev);
> > if (ret)
> > - goto free_dev;
> > + return ret;
> >
> > ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
> > if (ret)
> > @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > }
> > }
> >
> > - ret = qxl_device_init(qdev, _driver, pdev);
> > + ret = qxl_device_init(qdev, pdev);
> > if (ret)
> > goto put_vga;
> >
> > @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > disable_pci:
> > pci_disable_device(pdev);
> > -free_dev:
> > - kfree(qdev);
> > +
> > return ret;
> > }
> >
> > @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
> > drm_atomic_helper_shutdown(dev);
> > if (is_vga(pdev))
> > vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > - drm_dev_put(dev);
> > }
> >
> > DEFINE_DRM_GEM_FOPS(qxl_fops);
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> > index 435126facc9b..86ac191d9205 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.h
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> > @@ -276,8 +276,7 @@ struct qxl_device {
> > extern const struct drm_ioctl_desc qxl_ioctls[];
> > extern int qxl_max_ioctl;
> >
> > -int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
> > - struct pci_dev *pdev);
> > +int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
> > void qxl_device_fini(struct qxl_device *qdev);
> >
> > int qxl_modeset_init(struct qxl_device *qdev);
> > diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> > index 9eed1a375f24..91a34dd835d7 100644
> > --- a/drivers/gpu/drm/qxl/qxl_kms.c
> > +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> > @@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
> > }
> >
> > int qxl_device_init(struct qxl_device *qdev,
> > - struct drm_driver *drv,
> > struct pci_dev *pdev)
> > {
> > int r, sb;
> >
> > - r = drm_dev_init(>ddev, drv, >dev);
> > - if (r) {
> > - pr_err("Unable to init drm dev");
> > - goto error;
> > - }
> > -
> > qdev->ddev.pdev = pdev;
> > pci_set_drvdata(pdev, >ddev);
> > qdev->ddev.dev_private = qdev;
> > - drmm_add_final_kfree(>ddev, qdev);
> >
>
[Spice-devel] [PATCH 26/59] drm/qxl: Use devm_drm_dev_alloc
Also need to remove the drm_dev_put from the remove hook.
Acked-by: Gerd Hoffmann
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_drv.c | 15 ---
drivers/gpu/drm/qxl/qxl_drv.h | 3 +--
drivers/gpu/drm/qxl/qxl_kms.c | 12 +---
3 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 09102e2efabc..6b4ae4c5fb76 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
return -EINVAL; /* TODO: ENODEV ? */
}
- qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
- if (!qdev)
+ qdev = devm_drm_dev_alloc(>dev, _driver,
+ struct qxl_device, ddev);
+ if (IS_ERR(qdev)) {
+ pr_err("Unable to init drm dev");
return -ENOMEM;
+ }
ret = pci_enable_device(pdev);
if (ret)
- goto free_dev;
+ return ret;
ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
if (ret)
@@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
}
}
- ret = qxl_device_init(qdev, _driver, pdev);
+ ret = qxl_device_init(qdev, pdev);
if (ret)
goto put_vga;
@@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
vga_put(pdev, VGA_RSRC_LEGACY_IO);
disable_pci:
pci_disable_device(pdev);
-free_dev:
- kfree(qdev);
+
return ret;
}
@@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
drm_atomic_helper_shutdown(dev);
if (is_vga(pdev))
vga_put(pdev, VGA_RSRC_LEGACY_IO);
- drm_dev_put(dev);
}
DEFINE_DRM_GEM_FOPS(qxl_fops);
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 435126facc9b..86ac191d9205 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -276,8 +276,7 @@ struct qxl_device {
extern const struct drm_ioctl_desc qxl_ioctls[];
extern int qxl_max_ioctl;
-int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
- struct pci_dev *pdev);
+int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
void qxl_device_fini(struct qxl_device *qdev);
int qxl_modeset_init(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 9eed1a375f24..91a34dd835d7 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
}
int qxl_device_init(struct qxl_device *qdev,
- struct drm_driver *drv,
struct pci_dev *pdev)
{
int r, sb;
- r = drm_dev_init(>ddev, drv, >dev);
- if (r) {
- pr_err("Unable to init drm dev");
- goto error;
- }
-
qdev->ddev.pdev = pdev;
pci_set_drvdata(pdev, >ddev);
qdev->ddev.dev_private = qdev;
- drmm_add_final_kfree(>ddev, qdev);
mutex_init(>gem.mutex);
mutex_init(>update_area_mutex);
@@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base,
pci_resource_len(pdev, 0));
if (!qdev->vram_mapping) {
pr_err("Unable to create vram_mapping");
- r = -ENOMEM;
- goto error;
+ return -ENOMEM;
}
if (pci_resource_len(pdev, 4) > 0) {
@@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
io_mapping_free(qdev->surface_mapping);
vram_mapping_free:
io_mapping_free(qdev->vram_mapping);
-error:
return r;
}
--
2.25.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 27/59] drm/qxl: Don't use drm_device->dev_private
Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.
Acked-by: Gerd Hoffmann
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_debugfs.c | 7 +++
drivers/gpu/drm/qxl/qxl_display.c | 32 +++
drivers/gpu/drm/qxl/qxl_drv.c | 8
drivers/gpu/drm/qxl/qxl_drv.h | 4 ++--
drivers/gpu/drm/qxl/qxl_dumb.c| 2 +-
drivers/gpu/drm/qxl/qxl_gem.c | 2 +-
drivers/gpu/drm/qxl/qxl_ioctl.c | 14 +++---
drivers/gpu/drm/qxl/qxl_irq.c | 2 +-
drivers/gpu/drm/qxl/qxl_kms.c | 1 -
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
drivers/gpu/drm/qxl/qxl_ttm.c | 2 +-
12 files changed, 38 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c
b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 88123047fdd4..524d35b648d8 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -39,7 +39,7 @@ static int
qxl_debugfs_irq_received(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct qxl_device *qdev = node->minor->dev->dev_private;
+ struct qxl_device *qdev = to_qxl(node->minor->dev);
seq_printf(m, "%d\n", atomic_read(>irq_received));
seq_printf(m, "%d\n", atomic_read(>irq_received_display));
@@ -53,7 +53,7 @@ static int
qxl_debugfs_buffers_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct qxl_device *qdev = node->minor->dev->dev_private;
+ struct qxl_device *qdev = to_qxl(node->minor->dev);
struct qxl_bo *bo;
list_for_each_entry(bo, >gem.objects, list) {
@@ -83,8 +83,7 @@ void
qxl_debugfs_init(struct drm_minor *minor)
{
#if defined(CONFIG_DEBUG_FS)
- struct qxl_device *dev =
- (struct qxl_device *) minor->dev->dev_private;
+ struct qxl_device *dev = to_qxl(minor->dev);
drm_debugfs_create_files(qxl_debugfs_list, QXL_DEBUGFS_ENTRIES,
minor->debugfs_root, minor);
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 09583a08e141..1082cd5d2fd4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -221,7 +221,7 @@ static int qxl_add_mode(struct drm_connector *connector,
bool preferred)
{
struct drm_device *dev = connector->dev;
- struct qxl_device *qdev = dev->dev_private;
+ struct qxl_device *qdev = to_qxl(dev);
struct drm_display_mode *mode = NULL;
int rc;
@@ -242,7 +242,7 @@ static int qxl_add_mode(struct drm_connector *connector,
static int qxl_add_monitors_config_modes(struct drm_connector *connector)
{
struct drm_device *dev = connector->dev;
- struct qxl_device *qdev = dev->dev_private;
+ struct qxl_device *qdev = to_qxl(dev);
struct qxl_output *output = drm_connector_to_qxl_output(connector);
int h = output->index;
struct qxl_head *head;
@@ -310,7 +310,7 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc
*crtc,
const char *reason)
{
struct drm_device *dev = crtc->dev;
- struct qxl_device *qdev = dev->dev_private;
+ struct qxl_device *qdev = to_qxl(dev);
struct qxl_crtc *qcrtc = to_qxl_crtc(crtc);
struct qxl_head head;
int oldcount, i = qcrtc->index;
@@ -400,7 +400,7 @@ static int qxl_framebuffer_surface_dirty(struct
drm_framebuffer *fb,
unsigned int num_clips)
{
/* TODO: vmwgfx where this was cribbed from had locking. Why? */
- struct qxl_device *qdev = fb->dev->dev_private;
+ struct qxl_device *qdev = to_qxl(fb->dev);
struct drm_clip_rect norect;
struct qxl_bo *qobj;
bool is_primary;
@@ -462,7 +462,7 @@ static const struct drm_crtc_helper_funcs
qxl_crtc_helper_funcs = {
static int qxl_primary_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
{
- struct qxl_device *qdev = plane->dev->dev_private;
+ struct qxl_device *qdev = to_qxl(plane->dev);
struct qxl_bo *bo;
if (!state->crtc || !state->fb)
@@ -476,7 +476,7 @@ static int qxl_primary_atomic_check(struct drm_plane *plane,
static int qxl_primary_apply_cursor(struct drm_plane *plane)
{
struct drm_device *dev = plane->dev;
- struct qxl_device *qdev = dev->dev_private;
+ struct qxl_device *qdev = to_qxl(dev);
struct drm_framebuffer *fb = plane->state->
Re: [Spice-devel] [PATCH 30/44] drm/qxl: Use devm_drm_dev_alloc
On Mon, Apr 6, 2020 at 7:29 PM Thomas Zimmermann wrote:
>
>
>
> Am 03.04.20 um 15:58 schrieb Daniel Vetter:
> > Also need to remove the drm_dev_put from the remove hook.
> >
> > Signed-off-by: Daniel Vetter
> > Cc: Dave Airlie
> > Cc: Gerd Hoffmann
> > Cc: virtualizat...@lists.linux-foundation.org
> > Cc: spice-devel@lists.freedesktop.org
> > ---
> > drivers/gpu/drm/qxl/qxl_drv.c | 15 ---
> > drivers/gpu/drm/qxl/qxl_drv.h | 3 +--
> > drivers/gpu/drm/qxl/qxl_kms.c | 12 +---
> > 3 files changed, 10 insertions(+), 20 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > index 09102e2efabc..6b4ae4c5fb76 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > @@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > return -EINVAL; /* TODO: ENODEV ? */
> > }
> >
> > - qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
> > - if (!qdev)
> > + qdev = devm_drm_dev_alloc(>dev, _driver,
> > + struct qxl_device, ddev);
> > + if (IS_ERR(qdev)) {
> > + pr_err("Unable to init drm dev");
> > return -ENOMEM;
> > + }
>
> My feeling is that it is too early to allocate. Wouldn't it be better to
> first do the pdev and conflicting-fb stuff and allocate right before
> qxl_device_init() ?
It doesn't matter. I can reorder, or I can also not reorder, it'll
have 0 effects on cleanup (all done automatically) or correctness
(it's just a memory allocation, it doesn't do anything).
-Daniel
> Best regards
> Thomas
>
> >
> > ret = pci_enable_device(pdev);
> > if (ret)
> > - goto free_dev;
> > + return ret;
> >
> > ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
> > if (ret)
> > @@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > }
> > }
> >
> > - ret = qxl_device_init(qdev, _driver, pdev);
> > + ret = qxl_device_init(qdev, pdev);
> > if (ret)
> > goto put_vga;
> >
> > @@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> > pci_device_id *ent)
> > vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > disable_pci:
> > pci_disable_device(pdev);
> > -free_dev:
> > - kfree(qdev);
> > +
> > return ret;
> > }
> >
> > @@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
> > drm_atomic_helper_shutdown(dev);
> > if (is_vga(pdev))
> > vga_put(pdev, VGA_RSRC_LEGACY_IO);
> > - drm_dev_put(dev);
> > }
> >
> > DEFINE_DRM_GEM_FOPS(qxl_fops);
> > diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
> > index 435126facc9b..86ac191d9205 100644
> > --- a/drivers/gpu/drm/qxl/qxl_drv.h
> > +++ b/drivers/gpu/drm/qxl/qxl_drv.h
> > @@ -276,8 +276,7 @@ struct qxl_device {
> > extern const struct drm_ioctl_desc qxl_ioctls[];
> > extern int qxl_max_ioctl;
> >
> > -int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
> > - struct pci_dev *pdev);
> > +int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
> > void qxl_device_fini(struct qxl_device *qdev);
> >
> > int qxl_modeset_init(struct qxl_device *qdev);
> > diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> > index 9eed1a375f24..91a34dd835d7 100644
> > --- a/drivers/gpu/drm/qxl/qxl_kms.c
> > +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> > @@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
> > }
> >
> > int qxl_device_init(struct qxl_device *qdev,
> > - struct drm_driver *drv,
> > struct pci_dev *pdev)
> > {
> > int r, sb;
> >
> > - r = drm_dev_init(>ddev, drv, >dev);
> > - if (r) {
> > - pr_err("Unable to init drm dev");
> > - goto error;
> > - }
> > -
> > qdev->ddev.pdev = pdev;
> > pci_set_drvdata(pdev, >ddev);
> > qdev->ddev.dev_private = qdev;
> > - drmm_add_final_kfree(>ddev, qdev);
> >
> > mutex_init(>gem.mutex);
> > mutex_init(>update_area_mutex);
> > @@ -138,8 +130,7 @@ int qxl_d
[Spice-devel] [PATCH 30/44] drm/qxl: Use devm_drm_dev_alloc
Also need to remove the drm_dev_put from the remove hook.
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_drv.c | 15 ---
drivers/gpu/drm/qxl/qxl_drv.h | 3 +--
drivers/gpu/drm/qxl/qxl_kms.c | 12 +---
3 files changed, 10 insertions(+), 20 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 09102e2efabc..6b4ae4c5fb76 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -81,13 +81,16 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
return -EINVAL; /* TODO: ENODEV ? */
}
- qdev = kzalloc(sizeof(struct qxl_device), GFP_KERNEL);
- if (!qdev)
+ qdev = devm_drm_dev_alloc(>dev, _driver,
+ struct qxl_device, ddev);
+ if (IS_ERR(qdev)) {
+ pr_err("Unable to init drm dev");
return -ENOMEM;
+ }
ret = pci_enable_device(pdev);
if (ret)
- goto free_dev;
+ return ret;
ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "qxl");
if (ret)
@@ -101,7 +104,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
}
}
- ret = qxl_device_init(qdev, _driver, pdev);
+ ret = qxl_device_init(qdev, pdev);
if (ret)
goto put_vga;
@@ -128,8 +131,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
pci_device_id *ent)
vga_put(pdev, VGA_RSRC_LEGACY_IO);
disable_pci:
pci_disable_device(pdev);
-free_dev:
- kfree(qdev);
+
return ret;
}
@@ -155,7 +157,6 @@ qxl_pci_remove(struct pci_dev *pdev)
drm_atomic_helper_shutdown(dev);
if (is_vga(pdev))
vga_put(pdev, VGA_RSRC_LEGACY_IO);
- drm_dev_put(dev);
}
DEFINE_DRM_GEM_FOPS(qxl_fops);
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 435126facc9b..86ac191d9205 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -276,8 +276,7 @@ struct qxl_device {
extern const struct drm_ioctl_desc qxl_ioctls[];
extern int qxl_max_ioctl;
-int qxl_device_init(struct qxl_device *qdev, struct drm_driver *drv,
- struct pci_dev *pdev);
+int qxl_device_init(struct qxl_device *qdev, struct pci_dev *pdev);
void qxl_device_fini(struct qxl_device *qdev);
int qxl_modeset_init(struct qxl_device *qdev);
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 9eed1a375f24..91a34dd835d7 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -108,21 +108,13 @@ static void qxl_gc_work(struct work_struct *work)
}
int qxl_device_init(struct qxl_device *qdev,
- struct drm_driver *drv,
struct pci_dev *pdev)
{
int r, sb;
- r = drm_dev_init(>ddev, drv, >dev);
- if (r) {
- pr_err("Unable to init drm dev");
- goto error;
- }
-
qdev->ddev.pdev = pdev;
pci_set_drvdata(pdev, >ddev);
qdev->ddev.dev_private = qdev;
- drmm_add_final_kfree(>ddev, qdev);
mutex_init(>gem.mutex);
mutex_init(>update_area_mutex);
@@ -138,8 +130,7 @@ int qxl_device_init(struct qxl_device *qdev,
qdev->vram_mapping = io_mapping_create_wc(qdev->vram_base,
pci_resource_len(pdev, 0));
if (!qdev->vram_mapping) {
pr_err("Unable to create vram_mapping");
- r = -ENOMEM;
- goto error;
+ return -ENOMEM;
}
if (pci_resource_len(pdev, 4) > 0) {
@@ -293,7 +284,6 @@ int qxl_device_init(struct qxl_device *qdev,
io_mapping_free(qdev->surface_mapping);
vram_mapping_free:
io_mapping_free(qdev->vram_mapping);
-error:
return r;
}
--
2.25.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 31/44] drm/qxl: Don't use drm_device->dev_private
Upcasting using a container_of macro is more typesafe, faster and
easier for the compiler to optimize.
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_debugfs.c | 7 +++
drivers/gpu/drm/qxl/qxl_display.c | 32 +++
drivers/gpu/drm/qxl/qxl_drv.c | 8
drivers/gpu/drm/qxl/qxl_drv.h | 4 ++--
drivers/gpu/drm/qxl/qxl_dumb.c| 2 +-
drivers/gpu/drm/qxl/qxl_gem.c | 2 +-
drivers/gpu/drm/qxl/qxl_ioctl.c | 14 +++---
drivers/gpu/drm/qxl/qxl_irq.c | 2 +-
drivers/gpu/drm/qxl/qxl_kms.c | 1 -
drivers/gpu/drm/qxl/qxl_object.c | 2 +-
drivers/gpu/drm/qxl/qxl_release.c | 2 +-
drivers/gpu/drm/qxl/qxl_ttm.c | 2 +-
12 files changed, 38 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c
b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 88123047fdd4..524d35b648d8 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -39,7 +39,7 @@ static int
qxl_debugfs_irq_received(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct qxl_device *qdev = node->minor->dev->dev_private;
+ struct qxl_device *qdev = to_qxl(node->minor->dev);
seq_printf(m, "%d\n", atomic_read(>irq_received));
seq_printf(m, "%d\n", atomic_read(>irq_received_display));
@@ -53,7 +53,7 @@ static int
qxl_debugfs_buffers_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = (struct drm_info_node *) m->private;
- struct qxl_device *qdev = node->minor->dev->dev_private;
+ struct qxl_device *qdev = to_qxl(node->minor->dev);
struct qxl_bo *bo;
list_for_each_entry(bo, >gem.objects, list) {
@@ -83,8 +83,7 @@ void
qxl_debugfs_init(struct drm_minor *minor)
{
#if defined(CONFIG_DEBUG_FS)
- struct qxl_device *dev =
- (struct qxl_device *) minor->dev->dev_private;
+ struct qxl_device *dev = to_qxl(minor->dev);
drm_debugfs_create_files(qxl_debugfs_list, QXL_DEBUGFS_ENTRIES,
minor->debugfs_root, minor);
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 09583a08e141..1082cd5d2fd4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -221,7 +221,7 @@ static int qxl_add_mode(struct drm_connector *connector,
bool preferred)
{
struct drm_device *dev = connector->dev;
- struct qxl_device *qdev = dev->dev_private;
+ struct qxl_device *qdev = to_qxl(dev);
struct drm_display_mode *mode = NULL;
int rc;
@@ -242,7 +242,7 @@ static int qxl_add_mode(struct drm_connector *connector,
static int qxl_add_monitors_config_modes(struct drm_connector *connector)
{
struct drm_device *dev = connector->dev;
- struct qxl_device *qdev = dev->dev_private;
+ struct qxl_device *qdev = to_qxl(dev);
struct qxl_output *output = drm_connector_to_qxl_output(connector);
int h = output->index;
struct qxl_head *head;
@@ -310,7 +310,7 @@ static void qxl_crtc_update_monitors_config(struct drm_crtc
*crtc,
const char *reason)
{
struct drm_device *dev = crtc->dev;
- struct qxl_device *qdev = dev->dev_private;
+ struct qxl_device *qdev = to_qxl(dev);
struct qxl_crtc *qcrtc = to_qxl_crtc(crtc);
struct qxl_head head;
int oldcount, i = qcrtc->index;
@@ -400,7 +400,7 @@ static int qxl_framebuffer_surface_dirty(struct
drm_framebuffer *fb,
unsigned int num_clips)
{
/* TODO: vmwgfx where this was cribbed from had locking. Why? */
- struct qxl_device *qdev = fb->dev->dev_private;
+ struct qxl_device *qdev = to_qxl(fb->dev);
struct drm_clip_rect norect;
struct qxl_bo *qobj;
bool is_primary;
@@ -462,7 +462,7 @@ static const struct drm_crtc_helper_funcs
qxl_crtc_helper_funcs = {
static int qxl_primary_atomic_check(struct drm_plane *plane,
struct drm_plane_state *state)
{
- struct qxl_device *qdev = plane->dev->dev_private;
+ struct qxl_device *qdev = to_qxl(plane->dev);
struct qxl_bo *bo;
if (!state->crtc || !state->fb)
@@ -476,7 +476,7 @@ static int qxl_primary_atomic_check(struct drm_plane *plane,
static int qxl_primary_apply_cursor(struct drm_plane *plane)
{
struct drm_device *dev = plane->dev;
- struct qxl_device *qdev = dev->dev_private;
+ struct qxl_device *qdev = to_qxl(dev);
struct drm_framebuffer *fb = plane->state->fb;
struct qxl_crtc *qc
[Spice-devel] [PATCH 07/51] drm/qxl: Use drmm_add_final_kfree
With this we can drop the final kfree from the release function. Acked-by: Gerd Hoffmann Signed-off-by: Daniel Vetter Cc: Dave Airlie Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: spice-devel@lists.freedesktop.org --- drivers/gpu/drm/qxl/qxl_drv.c | 2 -- drivers/gpu/drm/qxl/qxl_kms.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 4fda3f9b29f4..09102e2efabc 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -144,8 +144,6 @@ static void qxl_drm_release(struct drm_device *dev) */ qxl_modeset_fini(qdev); qxl_device_fini(qdev); - dev->dev_private = NULL; - kfree(qdev); } static void diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index 70b20ee4741a..09d7b5f6d172 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -27,6 +27,7 @@ #include #include +#include #include #include "qxl_drv.h" @@ -121,6 +122,7 @@ int qxl_device_init(struct qxl_device *qdev, qdev->ddev.pdev = pdev; pci_set_drvdata(pdev, >ddev); qdev->ddev.dev_private = qdev; + drmm_add_final_kfree(>ddev, qdev); mutex_init(>gem.mutex); mutex_init(>update_area_mutex); -- 2.25.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 07/51] drm/qxl: Use drmm_add_final_kfree
With this we can drop the final kfree from the release function. Signed-off-by: Daniel Vetter Cc: Dave Airlie Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: spice-devel@lists.freedesktop.org --- drivers/gpu/drm/qxl/qxl_drv.c | 2 -- drivers/gpu/drm/qxl/qxl_kms.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 4fda3f9b29f4..09102e2efabc 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -144,8 +144,6 @@ static void qxl_drm_release(struct drm_device *dev) */ qxl_modeset_fini(qdev); qxl_device_fini(qdev); - dev->dev_private = NULL; - kfree(qdev); } static void diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index 70b20ee4741a..09d7b5f6d172 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -27,6 +27,7 @@ #include #include +#include #include #include "qxl_drv.h" @@ -121,6 +122,7 @@ int qxl_device_init(struct qxl_device *qdev, qdev->ddev.pdev = pdev; pci_set_drvdata(pdev, >ddev); qdev->ddev.dev_private = qdev; + drmm_add_final_kfree(>ddev, qdev); mutex_init(>gem.mutex); mutex_init(>update_area_mutex); -- 2.24.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 07/51] drm/qxl: Use drmm_add_final_kfree
With this we can drop the final kfree from the release function. Signed-off-by: Daniel Vetter Cc: Dave Airlie Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: spice-devel@lists.freedesktop.org --- drivers/gpu/drm/qxl/qxl_drv.c | 2 -- drivers/gpu/drm/qxl/qxl_kms.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 4fda3f9b29f4..09102e2efabc 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -144,8 +144,6 @@ static void qxl_drm_release(struct drm_device *dev) */ qxl_modeset_fini(qdev); qxl_device_fini(qdev); - dev->dev_private = NULL; - kfree(qdev); } static void diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index 70b20ee4741a..09d7b5f6d172 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -27,6 +27,7 @@ #include #include +#include #include #include "qxl_drv.h" @@ -121,6 +122,7 @@ int qxl_device_init(struct qxl_device *qdev, qdev->ddev.pdev = pdev; pci_set_drvdata(pdev, >ddev); qdev->ddev.dev_private = qdev; + drmm_add_final_kfree(>ddev, qdev); mutex_init(>gem.mutex); mutex_init(>update_area_mutex); -- 2.24.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 07/51] drm/qxl: Use drmm_add_final_kfree
With this we can drop the final kfree from the release function. Signed-off-by: Daniel Vetter Cc: Dave Airlie Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: spice-devel@lists.freedesktop.org --- drivers/gpu/drm/qxl/qxl_drv.c | 2 -- drivers/gpu/drm/qxl/qxl_kms.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 4fda3f9b29f4..09102e2efabc 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -144,8 +144,6 @@ static void qxl_drm_release(struct drm_device *dev) */ qxl_modeset_fini(qdev); qxl_device_fini(qdev); - dev->dev_private = NULL; - kfree(qdev); } static void diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index 70b20ee4741a..09d7b5f6d172 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -27,6 +27,7 @@ #include #include +#include #include #include "qxl_drv.h" @@ -121,6 +122,7 @@ int qxl_device_init(struct qxl_device *qdev, qdev->ddev.pdev = pdev; pci_set_drvdata(pdev, >ddev); qdev->ddev.dev_private = qdev; + drmm_add_final_kfree(>ddev, qdev); mutex_init(>gem.mutex); mutex_init(>update_area_mutex); -- 2.24.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 3/4] drm/mgag200: Use simple encoder
On Fri, Feb 21, 2020 at 08:00:57PM +0100, Sam Ravnborg wrote:
> Hi Thomas.
>
> On Fri, Feb 21, 2020 at 08:48:48AM +0100, Thomas Zimmermann wrote:
> > Hi Sam
> >
> > thanks for reviewing the patch set.
> >
> > Am 20.02.20 um 19:56 schrieb Sam Ravnborg:
> > > Hi Thomas.
> > >
> > > On Tue, Feb 18, 2020 at 09:48:14AM +0100, Thomas Zimmermann wrote:
> > >> The mgag200 driver uses an empty implementation for its encoder. Replace
> > >> the code with the generic simple encoder.
> > >>
> > >> v2:
> > >> * rebase onto new simple-encoder interface
> > >>
> > >> Signed-off-by: Thomas Zimmermann
> > >> ---
> > >> drivers/gpu/drm/mgag200/mgag200_drv.h | 7 ---
> > >> drivers/gpu/drm/mgag200/mgag200_mode.c | 61 ++
> > >> 2 files changed, 3 insertions(+), 65 deletions(-)
> > >>
> > >> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.h
> > >> b/drivers/gpu/drm/mgag200/mgag200_drv.h
> > >> index aa32aad222c2..9bb9e8e14539 100644
> > >> --- a/drivers/gpu/drm/mgag200/mgag200_drv.h
> > >> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.h
> > >> @@ -95,7 +95,6 @@
> > >> #define MATROX_DPMS_CLEARED (-1)
> > >>
> > >> #define to_mga_crtc(x) container_of(x, struct mga_crtc, base)
> > >> -#define to_mga_encoder(x) container_of(x, struct mga_encoder, base)
> > >> #define to_mga_connector(x) container_of(x, struct mga_connector, base)
> > >>
> > >> struct mga_crtc {
> > >> @@ -110,12 +109,6 @@ struct mga_mode_info {
> > >> struct mga_crtc *crtc;
> > >> };
> > >>
> > >> -struct mga_encoder {
> > >> -struct drm_encoder base;
> > >> -int last_dpms;
> > >> -};
> > >> -
> > >> -
> > >> struct mga_i2c_chan {
> > >> struct i2c_adapter adapter;
> > >> struct drm_device *dev;
> > >
> > > Any particular reason why the drm_encoder is not embedded in struct
> > > mga_device?
> > >
> > > I found it more elegant - like you did it for ast in the previous patch.
> >
> > I think I wanted something that uses drm_simple_encoder_create(). But I
> > can change that. The embedded variant is indeed better.
>
> You should consider to drop drm_simple_encoder_create() until there
> is a driver that really needs it.
Yeah +1 on only the _init version. The create version really should use
drmm_kzalloc I think, but we're not quite there yet :-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 08/52] drm/qxl: Use drmm_add_final_kfree
With this we can drop the final kfree from the release function. Signed-off-by: Daniel Vetter Cc: Dave Airlie Cc: Gerd Hoffmann Cc: virtualizat...@lists.linux-foundation.org Cc: spice-devel@lists.freedesktop.org --- drivers/gpu/drm/qxl/qxl_drv.c | 2 -- drivers/gpu/drm/qxl/qxl_kms.c | 2 ++ 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c index 4fda3f9b29f4..09102e2efabc 100644 --- a/drivers/gpu/drm/qxl/qxl_drv.c +++ b/drivers/gpu/drm/qxl/qxl_drv.c @@ -144,8 +144,6 @@ static void qxl_drm_release(struct drm_device *dev) */ qxl_modeset_fini(qdev); qxl_device_fini(qdev); - dev->dev_private = NULL; - kfree(qdev); } static void diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c index 70b20ee4741a..09d7b5f6d172 100644 --- a/drivers/gpu/drm/qxl/qxl_kms.c +++ b/drivers/gpu/drm/qxl/qxl_kms.c @@ -27,6 +27,7 @@ #include #include +#include #include #include "qxl_drv.h" @@ -121,6 +122,7 @@ int qxl_device_init(struct qxl_device *qdev, qdev->ddev.pdev = pdev; pci_set_drvdata(pdev, >ddev); qdev->ddev.dev_private = qdev; + drmm_add_final_kfree(>ddev, qdev); mutex_init(>gem.mutex); mutex_init(>update_area_mutex); -- 2.24.1 ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 2/2] drm/qxl: add drm_driver.release callback.
On Mon, Feb 10, 2020 at 12:37:52PM +0100, Gerd Hoffmann wrote:
> Move final cleanups to qxl_drm_release() callback.
> Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
>
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 26 +++---
> 1 file changed, 19 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 1d601f57a6ba..4fda3f9b29f4 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -34,6 +34,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -132,21 +133,30 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> return ret;
> }
>
> +static void qxl_drm_release(struct drm_device *dev)
> +{
> + struct qxl_device *qdev = dev->dev_private;
> +
> + /*
> + * TODO: qxl_device_fini() call should be in qxl_pci_remove(),
> + * reodering qxl_modeset_fini() + qxl_device_fini() calls is
> + * non-trivial though.
> + */
> + qxl_modeset_fini(qdev);
So the drm_mode_config_cleanup call in here belongs in ->release, but the
qxl_destroy_monitors_object feels like should be perfectly fine in the
remove hook. You might need to sprinkle a few drm_dev_enter/exit around to
protect code paths thought.
Aside from this lgtm, for the series
Acked-by: Daniel Vetter
And up to you whether you want to fix this or not really.
Btw for testing all these patches that add a ->release hook I think it'd
be good if the drm core checks that drm_device->dev is set to NULL, and
that we do this in the remove hook. Since that's guaranteed to be gone at
that point, so anything in ->release that still needs the device is
broken. Ofc maybe do that check only for drivers which have a ->release
hook, and we might need a few fixups.
Cheers, Daniel
> + qxl_device_fini(qdev);
> + dev->dev_private = NULL;
> + kfree(qdev);
> +}
> +
> static void
> qxl_pci_remove(struct pci_dev *pdev)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
> - struct qxl_device *qdev = dev->dev_private;
>
> drm_dev_unregister(dev);
> -
> - qxl_modeset_fini(qdev);
> - qxl_device_fini(qdev);
> + drm_atomic_helper_shutdown(dev);
> if (is_vga(pdev))
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> -
> - dev->dev_private = NULL;
> - kfree(qdev);
> drm_dev_put(dev);
> }
>
> @@ -279,6 +289,8 @@ static struct drm_driver qxl_driver = {
> .major = 0,
> .minor = 1,
> .patchlevel = 0,
> +
> + .release = qxl_drm_release,
> };
>
> static int __init qxl_init(void)
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/6] drm: Add drm_simple_encoder_{init, create}()
On Fri, Feb 07, 2020 at 03:36:49PM +0100, Noralf Trønnes wrote:
>
>
> Den 07.02.2020 09.41, skrev Thomas Zimmermann:
> > The simple-encoder helpers initialize an encoder with an empty
> > implementation. This covers the requirements of most of the existing
> > DRM drivers. A call to drm_simple_encoder_create() allocates and
> > initializes an encoder instance, a call to drm_simple_encoder_init()
> > initializes a pre-allocated instance.
> >
> > Signed-off-by: Thomas Zimmermann
> > ---
> > drivers/gpu/drm/drm_encoder.c | 116 ++
> > include/drm/drm_encoder.h | 10 +++
> > 2 files changed, 126 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
>
>
>
> > +/**
> > + * drm_simple_encoder_create - Allocate and initialize an encoder
> > + * @dev: drm device
> > + * @encoder_type: user visible type of the encoder
> > + * @name: printf style format string for the encoder name, or NULL for
> > + *default name
> > + *
> > + * Allocates and initialises an encoder that has no further functionality.
> > The
> > + * encoder will be released automatically.
> > + *
> > + * Returns:
> > + * The encoder on success, a pointer-encoder error code on failure.
> > + */
> > +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev,
> > + int encoder_type,
> > + const char *name, ...)
> > +{
> > + char *namestr = NULL;
> > + struct drm_encoder *encoder;
> > + int ret;
> > +
> > + encoder = devm_kzalloc(dev->dev, sizeof(*encoder), GFP_KERNEL);
>
> The encoder can outlive the devres if the device is unbound when
> userspace has open fds, so you can't use devm_ here.
Ah yes great catch. Rule of thumb: Never use devm_ for any drm_*
structure. It's wrong.
There's a todo somewhere to essentially create a drm_managed set of apis
where the cleanup matches the right lifetime - we need to only
free/release drm resource at drm_driver->release time.
-Daniel
>
> Noralf.
>
> > + if (!encoder)
> > + return ERR_PTR(-ENOMEM);
> > +
> > + if (name) {
> > + va_list ap;
> > +
> > + va_start(ap, name);
> > + namestr = kvasprintf(GFP_KERNEL, name, ap);
> > + va_end(ap);
> > + if (!namestr) {
> > + ret = -ENOMEM;
> > + goto err_devm_kfree;
> > + }
> > + }
> > +
> > + ret = __drm_encoder_init(dev, encoder,
> > +_simple_encoder_funcs_destroy,
> > +encoder_type, namestr);
> > + if (ret)
> > + goto err_kfree;
> > +
> > + return encoder;
> > +
> > +err_kfree:
> > + if (name)
> > + kfree(namestr);
> > +err_devm_kfree:
> > + devm_kfree(dev->dev, encoder);
> > + return ERR_PTR(ret);
> > +}
> > +EXPORT_SYMBOL(drm_simple_encoder_create);
> > +
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/6] drm: Add drm_simple_encoder_{init, create}()
On Fri, Feb 07, 2020 at 03:02:00PM +0100, Thomas Zimmermann wrote:
> Hi
>
> Am 07.02.20 um 14:37 schrieb Daniel Vetter:
> > On Fri, Feb 07, 2020 at 09:41:31AM +0100, Thomas Zimmermann wrote:
> >> The simple-encoder helpers initialize an encoder with an empty
> >> implementation. This covers the requirements of most of the existing
> >> DRM drivers. A call to drm_simple_encoder_create() allocates and
> >> initializes an encoder instance, a call to drm_simple_encoder_init()
> >> initializes a pre-allocated instance.
> >>
> >> Signed-off-by: Thomas Zimmermann
> >
> > This has quick a bit midlayer taste to it ... I think having this as a
> > helper would be cleaner ...
>
> How would such a helper roughly look like?
Essentially same code, but stuff the helper into drm-kms-helper.ko, then
make sure it still works. The separate kernel module makes sure that the
drm core and helper stuff aren't too close friends with each another :-)
Essentially like the simple display pipe helpers.
-Daniel
>
> Best regards
> Thomas
>
> >
> > The other bit is drm_encoder->possible_crtcs. If we do create a helper for
> > these, lets at least try to make them not suck too badly :-) Otherwise I
> > guess it would be time to officially document what exactly possible_crtcs
> > == 0 means from an uabi pov.
> > -Daniel
> >
> >> ---
> >> drivers/gpu/drm/drm_encoder.c | 116 ++
> >> include/drm/drm_encoder.h | 10 +++
> >> 2 files changed, 126 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c
> >> index ffe691a1bf34..1a65cab1f310 100644
> >> --- a/drivers/gpu/drm/drm_encoder.c
> >> +++ b/drivers/gpu/drm/drm_encoder.c
> >> @@ -178,6 +178,122 @@ int drm_encoder_init(struct drm_device *dev,
> >> }
> >> EXPORT_SYMBOL(drm_encoder_init);
> >>
> >> +static const struct drm_encoder_funcs drm_simple_encoder_funcs_cleanup = {
> >> + .destroy = drm_encoder_cleanup,
> >> +};
> >> +
> >> +/**
> >> + * drm_simple_encoder_init - Init a preallocated encoder
> >> + * @dev: drm device
> >> + * @funcs: callbacks for this encoder
> >> + * @encoder_type: user visible type of the encoder
> >> + * @name: printf style format string for the encoder name, or NULL
> >> + *for default name
> >> + *
> >> + * Initialises a preallocated encoder that has no further functionality.
> >> The
> >> + * encoder will be released automatically.
> >> + *
> >> + * Returns:
> >> + * Zero on success, error code on failure.
> >> + */
> >> +int drm_simple_encoder_init(struct drm_device *dev,
> >> + struct drm_encoder *encoder,
> >> + int encoder_type, const char *name, ...)
> >> +{
> >> + char *namestr = NULL;
> >> + int ret;
> >> +
> >> + if (name) {
> >> + va_list ap;
> >> +
> >> + va_start(ap, name);
> >> + namestr = kvasprintf(GFP_KERNEL, name, ap);
> >> + va_end(ap);
> >> + if (!namestr)
> >> + return -ENOMEM;
> >> + }
> >> +
> >> + ret = __drm_encoder_init(dev, encoder,
> >> + _simple_encoder_funcs_cleanup,
> >> + encoder_type, namestr);
> >> + if (ret)
> >> + goto err_kfree;
> >> +
> >> + return 0;
> >> +
> >> +err_kfree:
> >> + if (name)
> >> + kfree(namestr);
> >> + return ret;
> >> +}
> >> +EXPORT_SYMBOL(drm_simple_encoder_init);
> >> +
> >> +static void drm_encoder_destroy(struct drm_encoder *encoder)
> >> +{
> >> + struct drm_device *dev = encoder->dev;
> >> +
> >> + drm_encoder_cleanup(encoder);
> >> + devm_kfree(dev->dev, encoder);
> >> +}
> >> +
> >> +static const struct drm_encoder_funcs drm_simple_encoder_funcs_destroy = {
> >> + .destroy = drm_encoder_destroy,
> >> +};
> >> +
> >> +/**
> >> + * drm_simple_encoder_create - Allocate and initialize an encoder
> >> + * @dev: drm device
> >> + * @encoder_type: user visible type of the encoder
> >> + * @name: printf style format string for the encoder name, or NULL for
> >> + *default name
> >> + *
> >> + * Allocates
Re: [Spice-devel] [PATCH 2/6] drm: Add drm_simple_encoder_{init, create}()
: > + if (name) > + kfree(namestr); > +err_devm_kfree: > + devm_kfree(dev->dev, encoder); > + return ERR_PTR(ret); > +} > +EXPORT_SYMBOL(drm_simple_encoder_create); > + > /** > * drm_encoder_cleanup - cleans up an initialised encoder > * @encoder: encoder to cleanup > diff --git a/include/drm/drm_encoder.h b/include/drm/drm_encoder.h > index 5623994b6e9e..0214f6cf9de6 100644 > --- a/include/drm/drm_encoder.h > +++ b/include/drm/drm_encoder.h > @@ -190,6 +190,16 @@ int drm_encoder_init(struct drm_device *dev, >const struct drm_encoder_funcs *funcs, >int encoder_type, const char *name, ...); > > +__printf(4, 5) > +int drm_simple_encoder_init(struct drm_device *dev, > + struct drm_encoder *encoder, > + int encoder_type, const char *name, ...); > + > +__printf(3, 4) > +struct drm_encoder *drm_simple_encoder_create(struct drm_device *dev, > + int encoder_type, > + const char *name, ...); > + > /** > * drm_encoder_index - find the index of a registered encoder > * @encoder: encoder to find index for > -- > 2.25.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] drm/qxl: add drm_driver.release callback.
On Fri, Feb 07, 2020 at 01:14:05PM +0100, Gerd Hoffmann wrote:
> Move final cleanups to qxl_drm_release() callback.
> Add drm_atomic_helper_shutdown() call to qxl_pci_remove().
>
> Reorder calls in qxl_device_fini(). Cleaning up gem & ttm
> might trigger qxl commands, so we should do that before
> releaseing command rings.
>
> Signed-off-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 21 ++---
> drivers/gpu/drm/qxl/qxl_kms.c | 8
> 2 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 1d601f57a6ba..8044363ba0f2 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -34,6 +34,7 @@
> #include
>
> #include
> +#include
> #include
> #include
> #include
> @@ -132,21 +133,25 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> return ret;
> }
>
> +static void qxl_drm_release(struct drm_device *dev)
> +{
> + struct qxl_device *qdev = dev->dev_private;
> +
> + qxl_modeset_fini(qdev);
> + qxl_device_fini(qdev);
Same here, there's a few iounmap and io_mapping_free which I think should
be in the pci_remove hook.
-Daniel
> + dev->dev_private = NULL;
> + kfree(qdev);
> +}
> +
> static void
> qxl_pci_remove(struct pci_dev *pdev)
> {
> struct drm_device *dev = pci_get_drvdata(pdev);
> - struct qxl_device *qdev = dev->dev_private;
>
> drm_dev_unregister(dev);
> -
> - qxl_modeset_fini(qdev);
> - qxl_device_fini(qdev);
> + drm_atomic_helper_shutdown(dev);
> if (is_vga(pdev))
> vga_put(pdev, VGA_RSRC_LEGACY_IO);
> -
> - dev->dev_private = NULL;
> - kfree(qdev);
> drm_dev_put(dev);
> }
>
> @@ -279,6 +284,8 @@ static struct drm_driver qxl_driver = {
> .major = 0,
> .minor = 1,
> .patchlevel = 0,
> +
> + .release = qxl_drm_release,
> };
>
> static int __init qxl_init(void)
> diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
> index bfc1631093e9..70b20ee4741a 100644
> --- a/drivers/gpu/drm/qxl/qxl_kms.c
> +++ b/drivers/gpu/drm/qxl/qxl_kms.c
> @@ -299,12 +299,12 @@ void qxl_device_fini(struct qxl_device *qdev)
> {
> qxl_bo_unref(>current_release_bo[0]);
> qxl_bo_unref(>current_release_bo[1]);
> - flush_work(>gc_work);
> - qxl_ring_free(qdev->command_ring);
> - qxl_ring_free(qdev->cursor_ring);
> - qxl_ring_free(qdev->release_ring);
> qxl_gem_fini(qdev);
> qxl_bo_fini(qdev);
> + flush_work(>gc_work);
> + qxl_ring_free(qdev->command_ring);
> + qxl_ring_free(qdev->cursor_ring);
> + qxl_ring_free(qdev->release_ring);
> io_mapping_free(qdev->surface_mapping);
> io_mapping_free(qdev->vram_mapping);
> iounmap(qdev->ram_header);
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 2/3] drm: drop resource_id parameter from drm_fb_helper_remove_conflicting_pci_framebuffers
On Thu, Aug 22, 2019 at 11:06:44AM +0200, Gerd Hoffmann wrote:
> Not needed any more for remove_conflicting_pci_framebuffers calls.
>
> Signed-off-by: Gerd Hoffmann
Reviewed-by: Daniel Vetter
> ---
> include/drm/drm_fb_helper.h | 4 +---
> drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
> drivers/gpu/drm/bochs/bochs_drv.c | 2 +-
> drivers/gpu/drm/cirrus/cirrus.c | 2 +-
> drivers/gpu/drm/mgag200/mgag200_drv.c | 2 +-
> drivers/gpu/drm/qxl/qxl_drv.c | 2 +-
> drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
> drivers/gpu/drm/virtio/virtgpu_drv.c| 1 -
> 8 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index 5a5f4b1d8241..8dcc012ccbc8 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -539,18 +539,16 @@ drm_fb_helper_remove_conflicting_framebuffers(struct
> apertures_struct *a,
> /**
> * drm_fb_helper_remove_conflicting_pci_framebuffers - remove
> firmware-configured framebuffers for PCI devices
> * @pdev: PCI device
> - * @resource_id: index of PCI BAR configuring framebuffer memory
> * @name: requesting driver name
> *
> * This function removes framebuffer devices (eg. initialized by firmware)
> - * using memory range configured for @pdev's BAR @resource_id.
> + * using memory range configured for any of @pdev's memory bars.
> *
> * The function assumes that PCI device with shadowed ROM drives a primary
> * display and so kicks out vga16fb.
> */
> static inline int
> drm_fb_helper_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
> - int resource_id,
> const char *name)
> {
> int ret = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 98df55534a6d..6b96a5738e57 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -1031,7 +1031,7 @@ static int amdgpu_pci_probe(struct pci_dev *pdev,
> }
>
> /* Get rid of things like offb */
> - ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
> "amdgpudrmfb");
> + ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev,
> "amdgpudrmfb");
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/bochs/bochs_drv.c
> b/drivers/gpu/drm/bochs/bochs_drv.c
> index 770e1625d05e..3b9b0d9bbc14 100644
> --- a/drivers/gpu/drm/bochs/bochs_drv.c
> +++ b/drivers/gpu/drm/bochs/bochs_drv.c
> @@ -114,7 +114,7 @@ static int bochs_pci_probe(struct pci_dev *pdev,
> return -ENOMEM;
> }
>
> - ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
> "bochsdrmfb");
> + ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev,
> "bochsdrmfb");
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/cirrus/cirrus.c b/drivers/gpu/drm/cirrus/cirrus.c
> index 36a69aec8a4b..89d9e6fdeb8c 100644
> --- a/drivers/gpu/drm/cirrus/cirrus.c
> +++ b/drivers/gpu/drm/cirrus/cirrus.c
> @@ -532,7 +532,7 @@ static int cirrus_pci_probe(struct pci_dev *pdev,
> struct cirrus_device *cirrus;
> int ret;
>
> - ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
> "cirrusdrmfb");
> + ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev,
> "cirrusdrmfb");
> if (ret)
> return ret;
>
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c
> b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index afd9119b6cf1..4f9df3b93598 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -46,7 +46,7 @@ MODULE_DEVICE_TABLE(pci, pciidlist);
>
> static int mga_pci_probe(struct pci_dev *pdev, const struct pci_device_id
> *ent)
> {
> - drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0,
> "mgag200drmfb");
> + drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, "mgag200drmfb");
>
> return drm_get_pci_dev(pdev, ent, );
> }
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index c1802e01d9f6..2b726a51a302 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -83,7 +83,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct
> pci_device_id *ent)
> if (ret)
> goto free_dev;
>
> - ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl"
Re: [Spice-devel] [PATCH] gpu: drm: qxl: Fix possible null-pointer dereferences in qxl_crtc_atomic_flush()
On Thu, Jul 25, 2019 at 06:21:27PM +0800, Jia-Ju Bai wrote: > In qxl_crtc_atomic_flush(), there is an if statement on line 376 to > check whether crtc->state is NULL: > if (crtc->state && crtc->state->event) > > When crtc->state is NULL and qxl_crtc_update_monitors_config() is call, > qxl_crtc_update_monitors_config() uses crtc->state on line 326: > if (crtc->state->active) > and on line 358: > DRM_DEBUG_KMS(..., crtc->state->active, ...); > > Thus, possible null-pointer dereferences may occur. > > To fix these bugs, crtc->state is checked before calling > qxl_crtc_update_monitors_config(). > > These bugs are found by a static analysis tool STCheck written by us. > > Signed-off-by: Jia-Ju Bai crtc->state should never be NULL in this function, ever. Imo correct fix is to remove that other NULL check (since obviously it would blow up, hence it's dead code). Atomic kms drivers use drm_mode_config_reset() to make sure the various ->state pointers are always set and valid. -Daniel > --- > drivers/gpu/drm/qxl/qxl_display.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_display.c > b/drivers/gpu/drm/qxl/qxl_display.c > index 8b319ebbb0fb..fae18ef1ba59 100644 > --- a/drivers/gpu/drm/qxl/qxl_display.c > +++ b/drivers/gpu/drm/qxl/qxl_display.c > @@ -382,7 +382,8 @@ static void qxl_crtc_atomic_flush(struct drm_crtc *crtc, > spin_unlock_irqrestore(>event_lock, flags); > } > > - qxl_crtc_update_monitors_config(crtc, "flush"); > + if (crtc->state) > + qxl_crtc_update_monitors_config(crtc, "flush"); > } > > static void qxl_crtc_destroy(struct drm_crtc *crtc) > -- > 2.17.0 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH] drm/prime: Actually remove DRIVER_PRIME everywhere
Split out to make the functional changes stick out more.
v2: amdgpu gained DRIVER_SYNCOBJ_TIMELINE.
v3: amdgpu lost DRIVER_SYNCOBJ_TIMELINE.
v4: Don't add a space in i915_drv.c (Sam)
Cc: Sam Ravnborg
Reviewed-by: Eric Anholt
Signed-off-by: Daniel Vetter
Cc: amd-...@lists.freedesktop.org
Cc: etna...@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Cc: l...@lists.freedesktop.org
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: linux-asp...@lists.ozlabs.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-st...@st-md-mailman.stormreply.com
Cc: linux-te...@vger.kernel.org
Cc: nouv...@lists.freedesktop.org
Cc: NXP Linux Team
Cc: spice-devel@lists.freedesktop.org
Cc: virtualizat...@lists.linux-foundation.org
Cc: VMware Graphics
Cc: xen-de...@lists.xenproject.org
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
drivers/gpu/drm/arc/arcpgu_drv.c| 3 +--
drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 2 +-
drivers/gpu/drm/arm/hdlcd_drv.c | 4 +---
drivers/gpu/drm/arm/malidp_drv.c| 3 +--
drivers/gpu/drm/armada/armada_drv.c | 3 +--
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 3 +--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 4 +---
drivers/gpu/drm/bochs/bochs_drv.c | 3 +--
drivers/gpu/drm/cirrus/cirrus.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 4 +---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 +--
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 3 +--
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/imx/imx-drm-core.c | 3 +--
drivers/gpu/drm/lima/lima_drv.c | 2 +-
drivers/gpu/drm/mcde/mcde_drv.c | 2 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 3 +--
drivers/gpu/drm/meson/meson_drv.c | 4 +---
drivers/gpu/drm/msm/msm_drv.c | 1 -
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
drivers/gpu/drm/omapdrm/omap_drv.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_drv.c | 3 +--
drivers/gpu/drm/pl111/pl111_drv.c | 2 +-
drivers/gpu/drm/qxl/qxl_drv.c | 3 +--
drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 3 +--
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 3 +--
drivers/gpu/drm/shmobile/shmob_drm_drv.c| 3 +--
drivers/gpu/drm/sti/sti_drv.c | 3 +--
drivers/gpu/drm/stm/drv.c | 3 +--
drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +-
drivers/gpu/drm/tegra/drm.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +--
drivers/gpu/drm/tinydrm/hx8357d.c | 2 +-
drivers/gpu/drm/tinydrm/ili9225.c | 3 +--
drivers/gpu/drm/tinydrm/ili9341.c | 2 +-
drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +--
drivers/gpu/drm/tinydrm/repaper.c | 3 +--
drivers/gpu/drm/tinydrm/st7586.c| 3 +--
drivers/gpu/drm/tinydrm/st7735r.c | 3 +--
drivers/gpu/drm/tve200/tve200_drv.c | 3 +--
drivers/gpu/drm/udl/udl_drv.c | 2 +-
drivers/gpu/drm/v3d/v3d_drv.c | 1 -
drivers/gpu/drm/vboxvideo/vbox_drv.c| 2 +-
drivers/gpu/drm/vc4/vc4_drv.c | 1 -
drivers/gpu/drm/vgem/vgem_drv.c | 3 +--
drivers/gpu/drm/virtio/virtgpu_drv.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
drivers/gpu/drm/xen/xen_drm_front.c | 3 +--
drivers/gpu/drm/zte/zx_drm_drv.c| 3 +--
include/drm/drm_drv.h | 6 --
54 files changed, 50 insertions(+), 94 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0a577a389024..8e1b269351e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1309,7 +1309,7 @@ static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_ATOMIC |
DRIVER_GEM |
- DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
+ DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
.load = amdgpu_driver_load_kms,
.open = amdgpu_driver_open_kms,
.postclose = amdgpu_driver_postclose_kms,
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index af60c6d7a5f4..74240cc1c300 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -135,8 +135,7 @@ static int arcpgu_debugfs_init(struct
[Spice-devel] [PATCH 06/59] drm/prime: Actually remove DRIVER_PRIME everywhere
Split out to make the functional changes stick out more.
v2: amdgpu gained DRIVER_SYNCOBJ_TIMELINE.
v3: amdgpu lost DRIVER_SYNCOBJ_TIMELINE.
Signed-off-by: Daniel Vetter
Cc: amd-...@lists.freedesktop.org
Cc: etna...@lists.freedesktop.org
Cc: freedr...@lists.freedesktop.org
Cc: intel-...@lists.freedesktop.org
Cc: l...@lists.freedesktop.org
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: linux-asp...@lists.ozlabs.org
Cc: linux-renesas-...@vger.kernel.org
Cc: linux-rockc...@lists.infradead.org
Cc: linux-samsung-...@vger.kernel.org
Cc: linux-st...@st-md-mailman.stormreply.com
Cc: linux-te...@vger.kernel.org
Cc: nouv...@lists.freedesktop.org
Cc: NXP Linux Team
Cc: spice-devel@lists.freedesktop.org
Cc: virtualizat...@lists.linux-foundation.org
Cc: VMware Graphics
Cc: xen-de...@lists.xenproject.org
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
drivers/gpu/drm/arc/arcpgu_drv.c| 3 +--
drivers/gpu/drm/arm/display/komeda/komeda_kms.c | 2 +-
drivers/gpu/drm/arm/hdlcd_drv.c | 4 +---
drivers/gpu/drm/arm/malidp_drv.c| 3 +--
drivers/gpu/drm/armada/armada_drv.c | 3 +--
drivers/gpu/drm/aspeed/aspeed_gfx_drv.c | 3 +--
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 4 +---
drivers/gpu/drm/bochs/bochs_drv.c | 3 +--
drivers/gpu/drm/cirrus/cirrus.c | 2 +-
drivers/gpu/drm/etnaviv/etnaviv_drv.c | 4 +---
drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +-
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 3 +--
drivers/gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 3 +--
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/imx/imx-drm-core.c | 3 +--
drivers/gpu/drm/lima/lima_drv.c | 2 +-
drivers/gpu/drm/mcde/mcde_drv.c | 2 +-
drivers/gpu/drm/mediatek/mtk_drm_drv.c | 3 +--
drivers/gpu/drm/meson/meson_drv.c | 4 +---
drivers/gpu/drm/msm/msm_drv.c | 1 -
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
drivers/gpu/drm/omapdrm/omap_drv.c | 2 +-
drivers/gpu/drm/panfrost/panfrost_drv.c | 3 +--
drivers/gpu/drm/pl111/pl111_drv.c | 2 +-
drivers/gpu/drm/qxl/qxl_drv.c | 3 +--
drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
drivers/gpu/drm/rcar-du/rcar_du_drv.c | 3 +--
drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 3 +--
drivers/gpu/drm/shmobile/shmob_drm_drv.c| 3 +--
drivers/gpu/drm/sti/sti_drv.c | 3 +--
drivers/gpu/drm/stm/drv.c | 3 +--
drivers/gpu/drm/sun4i/sun4i_drv.c | 2 +-
drivers/gpu/drm/tegra/drm.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 3 +--
drivers/gpu/drm/tinydrm/hx8357d.c | 2 +-
drivers/gpu/drm/tinydrm/ili9225.c | 3 +--
drivers/gpu/drm/tinydrm/ili9341.c | 2 +-
drivers/gpu/drm/tinydrm/mi0283qt.c | 3 +--
drivers/gpu/drm/tinydrm/repaper.c | 3 +--
drivers/gpu/drm/tinydrm/st7586.c| 3 +--
drivers/gpu/drm/tinydrm/st7735r.c | 3 +--
drivers/gpu/drm/tve200/tve200_drv.c | 3 +--
drivers/gpu/drm/udl/udl_drv.c | 2 +-
drivers/gpu/drm/v3d/v3d_drv.c | 1 -
drivers/gpu/drm/vboxvideo/vbox_drv.c| 2 +-
drivers/gpu/drm/vc4/vc4_drv.c | 1 -
drivers/gpu/drm/vgem/vgem_drv.c | 3 +--
drivers/gpu/drm/virtio/virtgpu_drv.c| 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
drivers/gpu/drm/xen/xen_drm_front.c | 3 +--
drivers/gpu/drm/zte/zx_drm_drv.c| 3 +--
include/drm/drm_drv.h | 6 --
54 files changed, 50 insertions(+), 94 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0a577a389024..8e1b269351e8 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1309,7 +1309,7 @@ static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_ATOMIC |
DRIVER_GEM |
- DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
+ DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
.load = amdgpu_driver_load_kms,
.open = amdgpu_driver_open_kms,
.postclose = amdgpu_driver_postclose_kms,
diff --git a/drivers/gpu/drm/arc/arcpgu_drv.c b/drivers/gpu/drm/arc/arcpgu_drv.c
index af60c6d7a5f4..74240cc1c300 100644
--- a/drivers/gpu/drm/arc/arcpgu_drv.c
+++ b/drivers/gpu/drm/arc/arcpgu_drv.c
@@ -135,8 +135,7 @@ static int arcpgu_debugfs_init(struct drm_minor *minor)
#endif
static struct drm_driver arcpgu_drm_driver
[Spice-devel] [PATCH 25/59] drm/qxl: Drop drm_gem_prime_export/import
They're the default.
Aside: Would be really nice to switch the others over to
drm_gem_object_funcs.
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_drv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 61e1ce16fc25..d8f64886474b 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -256,8 +256,6 @@ static struct drm_driver qxl_driver = {
#endif
.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
- .gem_prime_export = drm_gem_prime_export,
- .gem_prime_import = drm_gem_prime_import,
.gem_prime_pin = qxl_gem_prime_pin,
.gem_prime_unpin = qxl_gem_prime_unpin,
.gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] drm/qxl: drop WARN_ONCE()
On Fri, May 24, 2019 at 12:42:50PM +0200, Gerd Hoffmann wrote:
> There is no good reason to flood the kernel log with a WARN
> stacktrace just because someone tried to mmap a prime buffer.
Yeah no userspace triggerable dmesg noise above debug level.
>
> Signed-off-by: Gerd Hoffmann
Reviewed-by: Daniel Vetter
> ---
> drivers/gpu/drm/qxl/qxl_prime.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
> index 114653b471c6..7d3816fca5a8 100644
> --- a/drivers/gpu/drm/qxl/qxl_prime.c
> +++ b/drivers/gpu/drm/qxl/qxl_prime.c
> @@ -77,6 +77,5 @@ void qxl_gem_prime_vunmap(struct drm_gem_object *obj, void
> *vaddr)
> int qxl_gem_prime_mmap(struct drm_gem_object *obj,
> struct vm_area_struct *area)
> {
> - WARN_ONCE(1, "not implemented");
> return -ENOSYS;
> }
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Revert "drm/qxl: drop prime import/export callbacks"
On Mon, Apr 29, 2019 at 4:38 PM Gerd Hoffmann wrote:
>
> Hi,
>
> > > More useful would be some way to signal this self-reimport capability
> > > to userspace somehow. See DRM_PRIME_CAP_LOCAL patch.
> >
> > Userspace is supposed to test whether import/export works for a
> > specific combo, not blindly assume it does and then keel over. I think
> > we need to fix that, not add more flags - there's lots of reasons why
> > a given pair of devices can't share buffers (e.g. all the contiguous
> > allocations stuff on socs).
>
> Ok. Lets scratch the DRM_PRIME_CAP_LOCAL idea then and blame userspace
> instead.
>
> > > Right now I have the choice to set DRM_PRIME_CAP_{IMPORT,EXPORT}, in
> > > which case some userspace assumes it can do cross-driver export/import
> > > and trips over that not working. Or I do not set
> > > DRM_PRIME_CAP_{IMPORT,EXPORT}, which breaks DRI3 ...
> >
> > Yeah that's not an option.
>
> Good. Can I get an ack for this patch then, as it unbreaks DRI3 with qxl?
Oh sure, acked-by: me. Sorry forgot to supply that ...
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Revert "drm/qxl: drop prime import/export callbacks"
On Mon, Apr 29, 2019 at 9:54 AM Gerd Hoffmann wrote:
>
> On Fri, Apr 26, 2019 at 04:21:37PM +0200, Daniel Vetter wrote:
> > On Fri, Apr 26, 2019 at 7:33 AM Gerd Hoffmann wrote:
> > >
> > > This reverts commit f4c34b1e2a37d5676180901fa6ff188bcb6371f8.
> > >
> > > Simliar to commit a0cecc23cfcb Revert "drm/virtio: drop prime
> > > import/export callbacks". We have to do the same with qxl,
> > > for the same reasons (it breaks DRI3).
> > >
> > > Drop the WARN_ON_ONCE().
> > >
> > > Fixes: f4c34b1e2a37d5676180901fa6ff188bcb6371f8
> > > Signed-off-by: Gerd Hoffmann
> >
> > Maybe we need some helpers for virtual drivers which only allow
> > self-reimport and nothing else at all?
>
> The current gem prime helpers handle this reasonable well,
> I don't see a need for that.
>
> More useful would be some way to signal this self-reimport capability
> to userspace somehow. See DRM_PRIME_CAP_LOCAL patch.
Userspace is supposed to test whether import/export works for a
specific combo, not blindly assume it does and then keel over. I think
we need to fix that, not add more flags - there's lots of reasons why
a given pair of devices can't share buffers (e.g. all the contiguous
allocations stuff on socs).
> Right now I have the choice to set DRM_PRIME_CAP_{IMPORT,EXPORT}, in
> which case some userspace assumes it can do cross-driver export/import
> and trips over that not working. Or I do not set
> DRM_PRIME_CAP_{IMPORT,EXPORT}, which breaks DRI3 ...
Yeah that's not an option.
-Daniel
> > I think there's qxl, virgl,
>
> export is implemented for virgl, and import should be possible too.
>
> qxl/bochs is simliar to vmgfx, it could be done but dma-bufs would
> basically be bounce buffers (need copy from/to vram) so probably not
> worth the effort.
>
> cheers,
> Gerd
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Revert "drm/qxl: drop prime import/export callbacks"
On Fri, Apr 26, 2019 at 05:42:23PM +0200, Thomas Hellstrom wrote:
> On 4/26/19 4:21 PM, Daniel Vetter wrote:
> > On Fri, Apr 26, 2019 at 7:33 AM Gerd Hoffmann wrote:
> > > This reverts commit f4c34b1e2a37d5676180901fa6ff188bcb6371f8.
> > >
> > > Simliar to commit a0cecc23cfcb Revert "drm/virtio: drop prime
> > > import/export callbacks". We have to do the same with qxl,
> > > for the same reasons (it breaks DRI3).
> > >
> > > Drop the WARN_ON_ONCE().
> > >
> > > Fixes: f4c34b1e2a37d5676180901fa6ff188bcb6371f8
> > > Signed-off-by: Gerd Hoffmann
> > Maybe we need some helpers for virtual drivers which only allow
> > self-reimport and nothing else at all? I think there's qxl, virgl,
> > vmwgfx and maybe also vbox one who could use this ... Just a quick
> > idea.
> > -Daniel
>
> I think vmwgfx could, in theory, support the full range of operations,
> at least for reasonably recent device versions. However, it wouldn't be
> terribly efficient since the exported dma-buf sglist would basically be a
> bounce-buffer.
Yeah not sure that makes sense to support really ...
-Daniel
>
> /Thomas
>
>
> > > ---
> > > drivers/gpu/drm/qxl/qxl_drv.c | 4
> > > drivers/gpu/drm/qxl/qxl_prime.c | 12
> > > 2 files changed, 16 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> > > index 578d867a81d5..f33e349c4ec5 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_drv.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> > > @@ -255,10 +255,14 @@ static struct drm_driver qxl_driver = {
> > > #if defined(CONFIG_DEBUG_FS)
> > > .debugfs_init = qxl_debugfs_init,
> > > #endif
> > > + .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> > > + .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> > > .gem_prime_export = drm_gem_prime_export,
> > > .gem_prime_import = drm_gem_prime_import,
> > > .gem_prime_pin = qxl_gem_prime_pin,
> > > .gem_prime_unpin = qxl_gem_prime_unpin,
> > > + .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
> > > + .gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
> > > .gem_prime_vmap = qxl_gem_prime_vmap,
> > > .gem_prime_vunmap = qxl_gem_prime_vunmap,
> > > .gem_prime_mmap = qxl_gem_prime_mmap,
> > > diff --git a/drivers/gpu/drm/qxl/qxl_prime.c
> > > b/drivers/gpu/drm/qxl/qxl_prime.c
> > > index 8b448eca1cd9..114653b471c6 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_prime.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_prime.c
> > > @@ -42,6 +42,18 @@ void qxl_gem_prime_unpin(struct drm_gem_object *obj)
> > > qxl_bo_unpin(bo);
> > > }
> > >
> > > +struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
> > > +{
> > > + return ERR_PTR(-ENOSYS);
> > > +}
> > > +
> > > +struct drm_gem_object *qxl_gem_prime_import_sg_table(
> > > + struct drm_device *dev, struct dma_buf_attachment *attach,
> > > + struct sg_table *table)
> > > +{
> > > + return ERR_PTR(-ENOSYS);
> > > +}
> > > +
> > > void *qxl_gem_prime_vmap(struct drm_gem_object *obj)
> > > {
> > > struct qxl_bo *bo = gem_to_qxl_bo(obj);
> > > --
> > > 2.18.1
> > >
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] Revert "drm/qxl: drop prime import/export callbacks"
On Fri, Apr 26, 2019 at 7:33 AM Gerd Hoffmann wrote:
>
> This reverts commit f4c34b1e2a37d5676180901fa6ff188bcb6371f8.
>
> Simliar to commit a0cecc23cfcb Revert "drm/virtio: drop prime
> import/export callbacks". We have to do the same with qxl,
> for the same reasons (it breaks DRI3).
>
> Drop the WARN_ON_ONCE().
>
> Fixes: f4c34b1e2a37d5676180901fa6ff188bcb6371f8
> Signed-off-by: Gerd Hoffmann
Maybe we need some helpers for virtual drivers which only allow
self-reimport and nothing else at all? I think there's qxl, virgl,
vmwgfx and maybe also vbox one who could use this ... Just a quick
idea.
-Daniel
> ---
> drivers/gpu/drm/qxl/qxl_drv.c | 4
> drivers/gpu/drm/qxl/qxl_prime.c | 12
> 2 files changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 578d867a81d5..f33e349c4ec5 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -255,10 +255,14 @@ static struct drm_driver qxl_driver = {
> #if defined(CONFIG_DEBUG_FS)
> .debugfs_init = qxl_debugfs_init,
> #endif
> + .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
> + .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
> .gem_prime_export = drm_gem_prime_export,
> .gem_prime_import = drm_gem_prime_import,
> .gem_prime_pin = qxl_gem_prime_pin,
> .gem_prime_unpin = qxl_gem_prime_unpin,
> + .gem_prime_get_sg_table = qxl_gem_prime_get_sg_table,
> + .gem_prime_import_sg_table = qxl_gem_prime_import_sg_table,
> .gem_prime_vmap = qxl_gem_prime_vmap,
> .gem_prime_vunmap = qxl_gem_prime_vunmap,
> .gem_prime_mmap = qxl_gem_prime_mmap,
> diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
> index 8b448eca1cd9..114653b471c6 100644
> --- a/drivers/gpu/drm/qxl/qxl_prime.c
> +++ b/drivers/gpu/drm/qxl/qxl_prime.c
> @@ -42,6 +42,18 @@ void qxl_gem_prime_unpin(struct drm_gem_object *obj)
> qxl_bo_unpin(bo);
> }
>
> +struct sg_table *qxl_gem_prime_get_sg_table(struct drm_gem_object *obj)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +
> +struct drm_gem_object *qxl_gem_prime_import_sg_table(
> + struct drm_device *dev, struct dma_buf_attachment *attach,
> + struct sg_table *table)
> +{
> + return ERR_PTR(-ENOSYS);
> +}
> +
> void *qxl_gem_prime_vmap(struct drm_gem_object *obj)
> {
> struct qxl_bo *bo = gem_to_qxl_bo(obj);
> --
> 2.18.1
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 0/5] Clean up TTM mmap offsets
On Mon, Mar 11, 2019 at 05:51:39PM +0100, Christian König wrote: > Am 11.03.19 um 17:39 schrieb Hans de Goede: > > Hi, > > > > On 07-02-19 09:59, Thomas Zimmermann wrote: > > > Almost all TTM-based drivers use the same values for the mmap-able > > > range of BO addresses. Each driver therefore duplicates the > > > DRM_FILE_PAGE_OFFSET constant. OTOH, the mmap range's size is not > > > configurable by drivers. > > > > > > This patch set replaces driver-specific configuration with a single > > > setup. All code is located within TTM. TTM and GEM share the same > > > range for mmap-able BOs. > > > > > > Thomas Zimmermann (5): > > > staging/vboxvideo: Use same BO mmap offset as other drivers > > > drm/ttm: Define a single DRM_FILE_PAGE_OFFSET constant > > > drm/ttm: Remove file_page_offset parameter from ttm_bo_device_init() > > > drm/ttm: Quick-test mmap offset in ttm_bo_mmap() > > > drm: Use the same mmap-range offset and size for GEM and TTM > > > > Note I'm about to push a patch-series to drm-misc-next which moves > > vboxvideo out of staging and I see that this series has not landed > > in drm-misc-next yet, so it will needs to be rebased. > > Mhm, TTM is usually not pushed upstream through drm-misc-next, so that will > certainly collide with the next TTM pull request. > > So can you wait with that or should I make an exception and merge this > change though drm-misc-next? Other options: - Get amdgpu added to drm-tip and linux-next so we have a heads-up about the conflict. That's usually good enough to avoid the broken merge conflict. - Do a topic branch, pull it into both trees. - Really stuff ttm into a shared tree if it's meant to be shared infrastructure :-P Waiting+rebasing is imo the worst option, and usually not needed. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v4 2/2] drm/qxl: remove conflicting framebuffers earlier
On Fri, Feb 22, 2019 at 08:16:04AM +0100, Gerd Hoffmann wrote: > Add error checking while being at it. > > Signed-off-by: Gerd Hoffmann Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/qxl/qxl_drv.c | 5 - > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c > index bb81e310eb6d..578d867a81d5 100644 > --- a/drivers/gpu/drm/qxl/qxl_drv.c > +++ b/drivers/gpu/drm/qxl/qxl_drv.c > @@ -79,6 +79,10 @@ qxl_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *ent) > if (ret) > goto free_dev; > > + ret = drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl"); > + if (ret) > + goto disable_pci; > + > ret = qxl_device_init(qdev, _driver, pdev); > if (ret) > goto disable_pci; > @@ -94,7 +98,6 @@ qxl_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *ent) > if (ret) > goto modeset_cleanup; > > - drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl"); > drm_fbdev_generic_setup(>ddev, 32); > return 0; > > -- > 2.9.3 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 2/2] drm/qxl: kick out vgacon
On Thu, Feb 21, 2019 at 4:11 PM Gerd Hoffmann wrote: > > Hi, > > > I was thinking of checking whether pdev is a VGA class device and whether > > it decodes vga access, and in that case automatically calling > > How can I figure that? Ok, class is easy, but decode? pci.h offers > functions to set vga decode but not to get that info ... PCI_COMMAND_MEM and PCI_COMMAND_IO. There doesn't seem to be any separate bits really. That's at least what I've gleaned from vgaarb.c. The magic legacy vga decode bits only seem to exist on bridges, maybe we can extract that logic from vgaarb.c (yes this is all a bit spiralling out of control). -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 2/2] drm/qxl: kick out vgacon
On Thu, Feb 21, 2019 at 02:06:23PM +0100, Gerd Hoffmann wrote: > On Thu, Feb 21, 2019 at 01:20:11PM +0100, Daniel Vetter wrote: > > On Thu, Feb 21, 2019 at 12:35:34PM +0100, Gerd Hoffmann wrote: > > > Problem: qxl switches from native mode back into vga compatibility mode > > > when it notices someone is accessing vga registers. And vgacon does > > > exactly that before fbcon takes over. So make sure we kick out vgacon > > > early enough that it wouldn't disturb us. > > > > > > Signed-off-by: Gerd Hoffmann > > > --- > > > drivers/gpu/drm/qxl/qxl_drv.c | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c > > > index bb81e310eb..08446561aa 100644 > > > --- a/drivers/gpu/drm/qxl/qxl_drv.c > > > +++ b/drivers/gpu/drm/qxl/qxl_drv.c > > > @@ -95,6 +95,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct > > > pci_device_id *ent) > > > goto modeset_cleanup; > > > > > > drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl"); > > > + drm_fb_helper_kick_out_vgacon(); > > > > I was thinking of checking whether pdev is a VGA class device and whether > > it decodes vga access, and in that case automatically calling > > kick_out_vgacon from remove_conflicting_pci_framebuffer. Since that's what > > drivers want anyway, and those who don't can open code it. > > > > Or is there an issue with that? > > It'll need more careful testing to make sure we don't have unwanted side > effects when just doing it for everyone. And I guess most drivers don't > care much because their hardware ignores vga port writes once they are > switched out of vga text mode. > > Dunno why i915 needs this. The problem isn't loading, it's unloading again. If you boot with vgacon, but no fbdev driver (which iirc also has magic code to throw out the vga console), then when you unload your kms driver vgacon kicks back in. And a pile of things go really sideways when that happens. I have no idea whether it's just intel hw or maybe pci decoding or something else, but seems like good practice to kick out all existing drivers, to make sure they can never get at the hw again. So don't think it'll hurt to do this for everyone. But yeah maybe we can do that as a follow-up (and convert i915 over), dunno. -Daniel > In case of qxl it is more a historical leftover. The very first qxl > device revision had no explicit qxl command to switch from qxl native > mode back to vga compatibility mode, vga port access was used for that > instead. It's long fixed, but the qxl device still has that quirk for > compatibility with very old guest drivers. > > cheers, > Gerd > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v2 2/2] drm/qxl: kick out vgacon
On Thu, Feb 21, 2019 at 12:35:34PM +0100, Gerd Hoffmann wrote: > Problem: qxl switches from native mode back into vga compatibility mode > when it notices someone is accessing vga registers. And vgacon does > exactly that before fbcon takes over. So make sure we kick out vgacon > early enough that it wouldn't disturb us. > > Signed-off-by: Gerd Hoffmann > --- > drivers/gpu/drm/qxl/qxl_drv.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c > index bb81e310eb..08446561aa 100644 > --- a/drivers/gpu/drm/qxl/qxl_drv.c > +++ b/drivers/gpu/drm/qxl/qxl_drv.c > @@ -95,6 +95,7 @@ qxl_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *ent) > goto modeset_cleanup; > > drm_fb_helper_remove_conflicting_pci_framebuffers(pdev, 0, "qxl"); > + drm_fb_helper_kick_out_vgacon(); I was thinking of checking whether pdev is a VGA class device and whether it decodes vga access, and in that case automatically calling kick_out_vgacon from remove_conflicting_pci_framebuffer. Since that's what drivers want anyway, and those who don't can open code it. Or is there an issue with that? -Daniel > drm_fbdev_generic_setup(>ddev, 32); > return 0; > > -- > 2.9.3 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] drm/qxl: unbind vgacon
On Wed, Feb 20, 2019 at 03:36:40PM +0100, Gerd Hoffmann wrote: > Problem: qxl switches from native mode back into vga compatibility mode > when it notices someone is accessing vga registers. And vgacon does > exactly that before fbcon takes over. > > Before qxl switched to the generic fbdev emulation that didn't cause any > problems. With the generic fbdev emulation the switch to vga mode > happens now and then, probably caused by a initialization order change > and triggered by a printk in a bad moment. > > So make sure we take vgacon out of the picture by making dummycon > taking over the console early enough. > > Not entriely happy with the approach, I'm open to better ideas. > > Signed-off-by: Gerd Hoffmann > --- > drivers/gpu/drm/qxl/qxl_drv.c | 6 ++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c > index bb81e310eb..88349dc13e 100644 > --- a/drivers/gpu/drm/qxl/qxl_drv.c > +++ b/drivers/gpu/drm/qxl/qxl_drv.c > @@ -30,6 +30,7 @@ > > #include > #include > +#include > > #include > #include > @@ -89,6 +90,11 @@ qxl_pci_probe(struct pci_dev *pdev, const struct > pci_device_id *ent) > > drm_kms_helper_poll_init(>ddev); > > + /* unbind vgacon to make sure it doesn't touch our vga registers */ > + console_lock(); > + ret = do_take_over_console(_con, 0, MAX_NR_CONSOLES - 1, true); > + console_unlock(); Still seems very late, in i915 we kick out vgacon as pretty much the first thing in driver load. See i915_kick_out_vgacon. I wonder whether we should integrate that logic into drm_fb_helper_remove_conflicting_pci_framebuffers, by checking whether that pci device can decode VGA and kick out vgacon in that case. Instead of sprinkling the same logic over all drivers. -Daniel > + > /* Complete initialization. */ > ret = drm_dev_register(>ddev, ent->driver_data); > if (ret) > -- > 2.9.3 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 1/3] drm/irq: Don't check for DRIVER_HAVE_IRQ in drm_irq_(un)install
If a non-legacy driver calls these it's valid to assume there is
interrupt support. The flag is really only needed for legacy drivers,
which control IRQ enabling/disabling through the DRM_IOCTL_CONTROL
legacy IOCTL.
Also remove all the flag usage from non-legacy drivers.
v2: Review from Emil:
- improve commit message
- I forgot hibmc, fix that
Cc: linux-arm-ker...@lists.infradead.org
Cc: intel-...@lists.freedesktop.org
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Cc: linux-renesas-...@vger.kernel.org
Reviewed-by: Emil Velikov
Reviewed-by: Sam Ravnborg
Signed-off-by: Daniel Vetter
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
drivers/gpu/drm/arm/hdlcd_drv.c | 2 +-
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c| 2 +-
drivers/gpu/drm/drm_irq.c | 6 --
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +-
drivers/gpu/drm/gma500/psb_drv.c| 2 +-
drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 3 +--
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/meson/meson_drv.c | 2 +-
drivers/gpu/drm/msm/msm_drv.c | 3 +--
drivers/gpu/drm/mxsfb/mxsfb_drv.c | 3 +--
drivers/gpu/drm/qxl/qxl_drv.c | 2 +-
drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
drivers/gpu/drm/shmobile/shmob_drm_drv.c| 2 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
drivers/gpu/drm/vc4/vc4_drv.c | 1 -
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
drivers/staging/vboxvideo/vbox_drv.c| 2 +-
18 files changed, 16 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0c22bae0c736..22502417c18c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1189,7 +1189,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev,
unsigned int pipe,
static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_ATOMIC |
- DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
+ DRIVER_IRQ_SHARED | DRIVER_GEM |
DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
.load = amdgpu_driver_load_kms,
.open = amdgpu_driver_open_kms,
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index e68935b80917..8fc0b884c428 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -229,7 +229,7 @@ static int hdlcd_debugfs_init(struct drm_minor *minor)
DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver hdlcd_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+ .driver_features = DRIVER_GEM |
DRIVER_MODESET | DRIVER_PRIME |
DRIVER_ATOMIC,
.irq_handler = hdlcd_irq,
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 034a91112098..0be13eceedba 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -720,7 +720,7 @@ static void atmel_hlcdc_dc_irq_uninstall(struct drm_device
*dev)
DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver atmel_hlcdc_dc_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+ .driver_features = DRIVER_GEM |
DRIVER_MODESET | DRIVER_PRIME |
DRIVER_ATOMIC,
.irq_handler = atmel_hlcdc_dc_irq_handler,
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 45a07652fa00..c5babb3e4752 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -103,9 +103,6 @@ int drm_irq_install(struct drm_device *dev, int irq)
int ret;
unsigned long sh_flags = 0;
- if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
- return -EOPNOTSUPP;
-
if (irq == 0)
return -EINVAL;
@@ -174,9 +171,6 @@ int drm_irq_uninstall(struct drm_device *dev)
bool irq_enabled;
int i;
- if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
- return -EOPNOTSUPP;
-
irq_enabled = dev->irq_enabled;
dev->irq_enabled = false;
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 54ace3436605..dfc73aade325 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -137,7 +137,7 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
static struct drm_driver fsl_dcu_drm_driver = {
- .driver_features= DRIVER_HA
[Spice-devel] [PATCH 01/26] drm/irq: Don't check for DRIVER_HAVE_IRQ in drm_irq_(un)install
If a non-legacy driver calls these it's valid to assume there is
interrupt support. The flag is really only needed for legacy drivers.
Also remove all the flag usage from non-legacy drivers.
Signed-off-by: Daniel Vetter
Cc: linux-arm-ker...@lists.infradead.org
Cc: intel-...@lists.freedesktop.org
Cc: linux-amlo...@lists.infradead.org
Cc: linux-arm-...@vger.kernel.org
Cc: freedr...@lists.freedesktop.org
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
Cc: amd-...@lists.freedesktop.org
Cc: linux-renesas-...@vger.kernel.org
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +-
drivers/gpu/drm/arm/hdlcd_drv.c | 2 +-
drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c | 2 +-
drivers/gpu/drm/drm_irq.c| 6 --
drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c| 2 +-
drivers/gpu/drm/gma500/psb_drv.c | 2 +-
drivers/gpu/drm/i915/i915_drv.c | 2 +-
drivers/gpu/drm/meson/meson_drv.c| 2 +-
drivers/gpu/drm/msm/msm_drv.c| 3 +--
drivers/gpu/drm/mxsfb/mxsfb_drv.c| 3 +--
drivers/gpu/drm/qxl/qxl_drv.c| 2 +-
drivers/gpu/drm/radeon/radeon_drv.c | 2 +-
drivers/gpu/drm/shmobile/shmob_drm_drv.c | 2 +-
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 2 +-
drivers/gpu/drm/vc4/vc4_drv.c| 1 -
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 2 +-
drivers/staging/vboxvideo/vbox_drv.c | 2 +-
17 files changed, 15 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 0c22bae0c736..22502417c18c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1189,7 +1189,7 @@ amdgpu_get_crtc_scanout_position(struct drm_device *dev,
unsigned int pipe,
static struct drm_driver kms_driver = {
.driver_features =
DRIVER_USE_AGP | DRIVER_ATOMIC |
- DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM |
+ DRIVER_IRQ_SHARED | DRIVER_GEM |
DRIVER_PRIME | DRIVER_RENDER | DRIVER_MODESET | DRIVER_SYNCOBJ,
.load = amdgpu_driver_load_kms,
.open = amdgpu_driver_open_kms,
diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index e68935b80917..8fc0b884c428 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -229,7 +229,7 @@ static int hdlcd_debugfs_init(struct drm_minor *minor)
DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver hdlcd_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+ .driver_features = DRIVER_GEM |
DRIVER_MODESET | DRIVER_PRIME |
DRIVER_ATOMIC,
.irq_handler = hdlcd_irq,
diff --git a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
index 034a91112098..0be13eceedba 100644
--- a/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
+++ b/drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.c
@@ -720,7 +720,7 @@ static void atmel_hlcdc_dc_irq_uninstall(struct drm_device
*dev)
DEFINE_DRM_GEM_CMA_FOPS(fops);
static struct drm_driver atmel_hlcdc_dc_driver = {
- .driver_features = DRIVER_HAVE_IRQ | DRIVER_GEM |
+ .driver_features = DRIVER_GEM |
DRIVER_MODESET | DRIVER_PRIME |
DRIVER_ATOMIC,
.irq_handler = atmel_hlcdc_dc_irq_handler,
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 45a07652fa00..c5babb3e4752 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -103,9 +103,6 @@ int drm_irq_install(struct drm_device *dev, int irq)
int ret;
unsigned long sh_flags = 0;
- if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
- return -EOPNOTSUPP;
-
if (irq == 0)
return -EINVAL;
@@ -174,9 +171,6 @@ int drm_irq_uninstall(struct drm_device *dev)
bool irq_enabled;
int i;
- if (!drm_core_check_feature(dev, DRIVER_HAVE_IRQ))
- return -EOPNOTSUPP;
-
irq_enabled = dev->irq_enabled;
dev->irq_enabled = false;
diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
index 54ace3436605..dfc73aade325 100644
--- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
+++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c
@@ -137,7 +137,7 @@ static irqreturn_t fsl_dcu_drm_irq(int irq, void *arg)
DEFINE_DRM_GEM_CMA_FOPS(fsl_dcu_drm_fops);
static struct drm_driver fsl_dcu_drm_driver = {
- .driver_features= DRIVER_HAVE_IRQ | DRIVER_GEM | DRIVER_MODESET
+ .driver_features= DRIVER_GEM | DRIVER_MODESET
| DRIVER_PRIME | DRIVER_ATOMIC,
.load = fsl_dcu_load,
.unload = fsl_dcu_unload,
diff --git a/drivers/gpu/drm/gma500/psb_drv.c b/drivers/gpu/drm/
[Spice-devel] [PATCH 19/26] drm/qxl: Use drm_fb_helper_fill_info
This should not result in any changes.
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_fb.c | 9 +
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_fb.c b/drivers/gpu/drm/qxl/qxl_fb.c
index d028471597ef..22dc9a7a643b 100644
--- a/drivers/gpu/drm/qxl/qxl_fb.c
+++ b/drivers/gpu/drm/qxl/qxl_fb.c
@@ -219,8 +219,6 @@ static int qxlfb_create(struct drm_fb_helper *helper,
goto out_unref;
}
- info->par = helper;
-
fb = drm_gem_fbdev_fb_create(>ddev, sizes, 64, gobj,
_fb_funcs);
if (IS_ERR(fb)) {
@@ -232,10 +230,6 @@ static int qxlfb_create(struct drm_fb_helper *helper,
/* setup helper with fb data */
qdev->fb_helper.fb = fb;
- strcpy(info->fix.id, "qxldrmfb");
-
- drm_fb_helper_fill_fix(info, fb->pitches[0], fb->format->depth);
-
info->fbops = _ops;
/*
@@ -247,8 +241,7 @@ static int qxlfb_create(struct drm_fb_helper *helper,
info->screen_base = shadow;
info->screen_size = gobj->size;
- drm_fb_helper_fill_var(info, >fb_helper, sizes->fb_width,
- sizes->fb_height);
+ drm_fb_helper_fill_info(info, >fb_helper);
/* setup aperture base/size for vesafb takeover */
info->apertures->ranges[0].base = qdev->ddev.mode_config.fb_base;
--
2.20.1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 19/26] drm/qxl: Use drm_fb_helper_fill_info
On Fri, Jan 25, 2019 at 07:39:26AM +0100, Gerd Hoffmann wrote: > On Thu, Jan 24, 2019 at 05:58:24PM +0100, Daniel Vetter wrote: > > This should not result in any changes. > > I'd love to merge https://patchwork.freedesktop.org/series/53951/ > instead (which will -- among other things -- switch qxl over to the > generic fbdev emulation and remove the code you are patching here). Ah yes very happy to drop my patch in favour of yours. > Anyone willing to review? Dave? Noralf might be a good choice too, with you cross-reviewing some of his in-flight work. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] drm: Split out drm_probe_helper.h
On Thu, Jan 24, 2019 at 10:46:47AM +0100, Daniel Vetter wrote: > On Wed, Jan 23, 2019 at 06:00:15PM +0100, Sam Ravnborg wrote: > > Hi Daniel. > > > > On Thu, Jan 17, 2019 at 10:03:34PM +0100, Daniel Vetter wrote: > > > Having the probe helper stuff (which pretty much everyone needs) in > > > the drm_crtc_helper.h file (which atomic drivers should never need) is > > > confusing. Split them out. > > > > > > To make sure I actually achieved the goal here I went through all > > > drivers. And indeed, all atomic drivers are now free of > > > drm_crtc_helper.h includes. > > > > How are the plans to get this patchset merged? > > There were dependencies on onging drmP.h removal in i915 IIRC? > > I guess my "Minimize drmP.h dependencies" patch-set also have a role in > > this. > > Working on it, a pull request got lost. I originally wanted to get this > all resolved this week, probably going to be next week now :-( We're lucky, all sorted now & patch merged. I'm also merging your series, correctly interleaved. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] drm: Split out drm_probe_helper.h
On Wed, Jan 23, 2019 at 06:00:15PM +0100, Sam Ravnborg wrote: > Hi Daniel. > > On Thu, Jan 17, 2019 at 10:03:34PM +0100, Daniel Vetter wrote: > > Having the probe helper stuff (which pretty much everyone needs) in > > the drm_crtc_helper.h file (which atomic drivers should never need) is > > confusing. Split them out. > > > > To make sure I actually achieved the goal here I went through all > > drivers. And indeed, all atomic drivers are now free of > > drm_crtc_helper.h includes. > > How are the plans to get this patchset merged? > There were dependencies on onging drmP.h removal in i915 IIRC? > I guess my "Minimize drmP.h dependencies" patch-set also have a role in this. Working on it, a pull request got lost. I originally wanted to get this all resolved this week, probably going to be next week now :-( -Daniel > This does not hold up any work of mine, mainly wanted to make > sure this was not lost between all the other patches. > > Sam -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] drm: Split out drm_probe_helper.h
On Mon, Jan 21, 2019 at 11:13 PM Sam Ravnborg wrote: > > Hi Daniel et al. > > > > > > > Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy > > > kms drivers. Just removing it from all the atomic drivers caused lots of > > > fallout, I expect even more if you entirely remove the includes it has. > > > Maybe a todo, care to pls create that patch since it's your idea? > > > > The main reason I bailed out initially was that this would create > > small changes to several otherwise seldomly touched files. > > And then we would later come and remove drmP.h - so lots of > > small but incremental changes to the same otherwise seldomly > > edited files. > > And the job was only partially done. > > > > I will try to experiment with an approach where I clean up the > > include/drm/*.h files a little (like suggested above, +delete drmP.h > > and maybe a bit more). > > > > Then to try on a driver by driver basis to make it build with a > > cleaned set of include files. > > I hope that the cleaned up driver can still build without the > > cleaned header files so the changes can be submitted piecemal. > > > > Will do so with an eye on the lesser maintained drivers to try it > > out to avoid creating too much chrunch for others. > > I have now a few patches queued, but the result is not too pretty. > I did the following: > > - For all files in include/drm/*.h the set of include files > were adjusted to the minimum number of files required to make > them build without any other files included first. > > Created one .c file for each .h file. Then included the .h > file and adjusted to the minimal set of include files. > In the process a lot of forwards were added. > > - Deleted drmP.h > > - Fixed build of a few drivers: sti, tilcdc, gma500, tve200, via > > Some observations: > > - Killing all the includes not needed in the headers files > results in a a lot of extra changes. > Examples: > drm_modseset_helper_vtables.h is no longer > included by anyone, so needs to be added in many files > > drm_atomic_state_helper.h is no longer included > by anyone so likewise needs to be added in many files > > - It is very tedious to do this properly. > The process I followed was: > - delete / comment out all include files > - add back the obvious from a quick scan of the code > - build - fix - build - fix - build - fix ... > - next file... > > - The result is errorprone as only the allyesconfig + allmodconfig > variants are tested. But reallife configurations are more diverse. > > Current diffstat: >111 files changed, 771 insertions(+), 401 deletions(-) > > This is for the 5 drivers alone and not the header cleanup. > So long story short - this is not good and not the way forward. > > I will try to come up with a few improvements to make the > headers files selfcontained, but restricted to the changes that > add forwards/include to avoid the chrunch in all the drivers. > > And then post for review a few patches to clean up some headers. > If the cleanup gets a go I will try to persuade the introduction > of these. > This will include, but will not be limited to, the above mentioned > drm_crtc_helper.h header file. > > For now too much time was already spent on this, so it is at the > moment pushed back on my TODO list. > This mail serve also as a kind of "where had I left", when/if I > pick this up again. > > If there are anyone that knows some tooling that can help in the > process of adjusting the header files I am all ears. Yeah in the process of splitting up drmP.h we've created a few smaller such piles of headers. I think in some cases it's just not going to be worth it to fully split them up, e.g. drm_crtc_helper.h is going to be a pure legacy helper, only needed by pre-atomic drivers. Splitting that up doesn't seem to useful to me. Similarly we might want drm_atomic_helper.h to keep pulling in the other helper headers. So probably going to be a judgement call on a case-by-case basis. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 23/23] drm/qxl: add overflow checks to qxl_mode_dumb_create()
On Fri, Jan 18, 2019 at 5:32 PM Ville Syrjälä
wrote:
>
> On Fri, Jan 18, 2019 at 04:49:44PM +0100, Daniel Vetter wrote:
> > On Fri, Jan 18, 2019 at 01:20:20PM +0100, Gerd Hoffmann wrote:
> > > Signed-off-by: Gerd Hoffmann
> >
> > We already do all reasonable overflow checks in drm_mode_create_dumb(). If
> > you don't trust them I think would be better time spent typing an igt to
> > test this than adding redundant check in all drivers.
> >
> > You're also missing one check for bpp underflows :-)
>
> BTW I just noticed that we don't seem to validating
> create_dumb->flags at all. Someone should probably add some
> checks for that, or mark it as deprecated in case we already
> lost the battle with userspace stack garbage.
Given that every kms client/compositor under the sun uses this (or
well, all the generic ones at least) I think we can safely assume to
have lost that battle :-/
-Daniel
>
> > -Daniel
> >
> > > ---
> > > drivers/gpu/drm/qxl/qxl_dumb.c | 10 ++
> > > 1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c
> > > b/drivers/gpu/drm/qxl/qxl_dumb.c
> > > index 272d19b677..bed6d06ee4 100644
> > > --- a/drivers/gpu/drm/qxl/qxl_dumb.c
> > > +++ b/drivers/gpu/drm/qxl/qxl_dumb.c
> > > @@ -37,11 +37,13 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
> > > uint32_t handle;
> > > int r;
> > > struct qxl_surface surf;
> > > - uint32_t pitch, format;
> > > + uint32_t pitch, size, format;
> > >
> > > - pitch = args->width * ((args->bpp + 1) / 8);
> > > - args->size = pitch * args->height;
> > > - args->size = ALIGN(args->size, PAGE_SIZE);
> > > + if (check_mul_overflow(args->width, ((args->bpp + 1) / 8), ))
> > > + return -EINVAL;
> > > + if (check_mul_overflow(pitch, args->height, ))
> > > + return -EINVAL;
> > > + args->size = ALIGN(size, PAGE_SIZE);
> > >
> > > switch (args->bpp) {
> > > case 16:
> > > --
> > > 2.9.3
> > >
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Ville Syrjälä
> Intel
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH v3 23/23] drm/qxl: add overflow checks to qxl_mode_dumb_create()
On Fri, Jan 18, 2019 at 01:20:20PM +0100, Gerd Hoffmann wrote:
> Signed-off-by: Gerd Hoffmann
We already do all reasonable overflow checks in drm_mode_create_dumb(). If
you don't trust them I think would be better time spent typing an igt to
test this than adding redundant check in all drivers.
You're also missing one check for bpp underflows :-)
-Daniel
> ---
> drivers/gpu/drm/qxl/qxl_dumb.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c
> index 272d19b677..bed6d06ee4 100644
> --- a/drivers/gpu/drm/qxl/qxl_dumb.c
> +++ b/drivers/gpu/drm/qxl/qxl_dumb.c
> @@ -37,11 +37,13 @@ int qxl_mode_dumb_create(struct drm_file *file_priv,
> uint32_t handle;
> int r;
> struct qxl_surface surf;
> - uint32_t pitch, format;
> + uint32_t pitch, size, format;
>
> - pitch = args->width * ((args->bpp + 1) / 8);
> - args->size = pitch * args->height;
> - args->size = ALIGN(args->size, PAGE_SIZE);
> + if (check_mul_overflow(args->width, ((args->bpp + 1) / 8), ))
> + return -EINVAL;
> + if (check_mul_overflow(pitch, args->height, ))
> + return -EINVAL;
> + args->size = ALIGN(size, PAGE_SIZE);
>
> switch (args->bpp) {
> case 16:
> --
> 2.9.3
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH] drm: Split out drm_probe_helper.h
On Wed, Jan 16, 2019 at 07:10:18PM +0100, Sam Ravnborg wrote: > Hi Daniel. > > > v5: Actually try to sort them, and while at it, sort all the ones I > > touch. > > Applied this variant on top of drm-misc and did a build test. > Looked good for ia64, x86 and alpha. > > Took a closer look at the changes to atmel_hlcd - and they looked OK. > > But I noticed that atmel_hlcdc uses only drm_kms_helper_poll_init() and > drm_kms_helper_poll_fini(). > But there are no hits on DRM_CONNECTOR_POLL - so I think we maybe > have a driver here where we have plugged the drm_poll infrastructure, > but it is not in use. > > > include/drm/drm_crtc_helper.h | 16 --- > > The list of include files in this file could be dropped and replaced by: > struct drm_connector; > struct drm_device; > struct drm_display_mode; > struct drm_encoder; > struct drm_framebuffer; > struct drm_mode_set; > struct drm_modeset_acquire_ctx; > > I tried to do so on top of your patch. > But there were too many build errros and I somehow lost the motivation. Yeah the drm_crtc_helper.h header is a bit the miniature drmP.h for legacy kms drivers. Just removing it from all the atomic drivers caused lots of fallout, I expect even more if you entirely remove the includes it has. Maybe a todo, care to pls create that patch since it's your idea? -Daniel > > > > include/drm/drm_probe_helper.h| 27 +++ > This on the other hand is fine - as expected as this is a new file. > > But the above is just some random comments so: > > Acked-by: Sam Ravnborg -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 1/3] drm/qxl: add mode/framebuffer check functions
On Wed, Jan 16, 2019 at 12:28:00PM +0100, Gerd Hoffmann wrote:
> > > +static int qxl_check_mode(struct qxl_device *qdev,
> > > + unsigned int width,
> > > + unsigned int height)
> > > +{
> > > + if (width * height * 4 > qdev->vram_size)
> >
> > Is someone checking for integer overflows already?
>
> Need to have a look. This is just ...
The addfb ioctl checks for integer overflows for you.
-Daniel
>
> > > - if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
> > > - DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
> > > - return -EINVAL;
> > > - }
>
> ... that check moved into the new function.
>
> cheers,
> Gerd
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
Re: [Spice-devel] [PATCH 7/7] drm: Split out drm_probe_helper.h
On Mon, Dec 10, 2018 at 02:40:25PM +0100, Benjamin Gaignard wrote: > Le lun. 10 déc. 2018 à 12:10, Benjamin Gaignard > a écrit : > > > > Le lun. 10 déc. 2018 à 11:24, Thierry Reding > > a écrit : > > > > > > On Mon, Dec 10, 2018 at 11:11:33AM +0100, Daniel Vetter wrote: > > > > Having the probe helper stuff (which pretty much everyone needs) in > > > > the drm_crtc_helper.h file (which atomic drivers should never need) is > > > > confusing. Split them out. > > > > > > > > To make sure I actually achieved the goal here I went through all > > > > drivers. And indeed, all atomic drivers are now free of > > > > drm_crtc_helper.h includes. > > > > > > > > I have difficulties to apply this with git on top of drm-misc-next. > > It is because of that I got errors (encoder and connector types not > > found) while compiling adv7511_audio.c and exynos_dp.c ? > > > > Nack on this patch because it break compiling at least on sti driver. > drm_probe_helper.h doesn't bring the same includes than drm_crtc_helper.h: > #include > #include > #include > so some types, structures and functions proptotypes are missing while > compiling. Hm, I thought I've compile-tested all the arm stuff, I guess I've failed. Will respin, sorry for the confusion. -Daniel > > > > Benjamin > > > > Signed-off-by: Daniel Vetter > > > > Cc: linux-arm-ker...@lists.infradead.org > > > > Cc: virtualizat...@lists.linux-foundation.org > > > > Cc: etna...@lists.freedesktop.org > > > > Cc: linux-samsung-...@vger.kernel.org > > > > Cc: intel-...@lists.freedesktop.org > > > > Cc: linux-media...@lists.infradead.org > > > > Cc: linux-amlo...@lists.infradead.org > > > > Cc: linux-arm-...@vger.kernel.org > > > > Cc: freedr...@lists.freedesktop.org > > > > Cc: nouv...@lists.freedesktop.org > > > > Cc: spice-devel@lists.freedesktop.org > > > > Cc: amd-...@lists.freedesktop.org > > > > Cc: linux-renesas-...@vger.kernel.org > > > > Cc: linux-rockc...@lists.infradead.org > > > > Cc: linux-st...@st-md-mailman.stormreply.com > > > > Cc: linux-te...@vger.kernel.org > > > > Cc: xen-de...@lists.xen.org > > > > --- > > > > .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 2 +- > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 2 +- > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- > > > > drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 + > > > > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- > > > > .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 2 +- > > > > .../display/amdgpu_dm/amdgpu_dm_services.c| 2 +- > > > > drivers/gpu/drm/arc/arcpgu_crtc.c | 2 +- > > > > drivers/gpu/drm/arc/arcpgu_drv.c | 2 +- > > > > drivers/gpu/drm/arc/arcpgu_sim.c | 2 +- > > > > drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +- > > > > drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- > > > > drivers/gpu/drm/arm/malidp_crtc.c | 2 +- > > > > drivers/gpu/drm/arm/malidp_drv.c | 2 +- > > > > drivers/gpu/drm/arm/malidp_mw.c | 2 +- > > > > drivers/gpu/drm/armada/armada_510.c | 2 +- > > > > drivers/gpu/drm/armada/armada_crtc.c | 2 +- > > > > drivers/gpu/drm/armada/armada_drv.c | 2 +- > > > > drivers/gpu/drm/armada/armada_fb.c| 2 +- > > > > drivers/gpu/drm/ast/ast_drv.c | 1 + > > > > drivers/gpu/drm/ast/ast_mode.c| 1 + > > > > .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 2 +- > > > > drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 +- > > > > drivers/gpu/drm/bochs/bochs_drv.c | 1 + > > > > drivers/gpu/drm/bochs/bochs_kms.c | 1 + > > > > drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 +- > > > > drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 +- > > > > .../drm/bridge/analogix/analogix_dp_core.c| 2 +- > > > > drivers/gpu/drm/bridge/cdns-dsi.c | 2 +- > > > > drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +- > > > > .../bridge/megachips-stdp-ge-b850v3-fw.c | 2 +- > > > > drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +- > > > > drivers/gpu/drm/bridge/panel.c
[Spice-devel] [PATCH 7/7] drm: Split out drm_probe_helper.h
Having the probe helper stuff (which pretty much everyone needs) in the drm_crtc_helper.h file (which atomic drivers should never need) is confusing. Split them out. To make sure I actually achieved the goal here I went through all drivers. And indeed, all atomic drivers are now free of drm_crtc_helper.h includes. Signed-off-by: Daniel Vetter Cc: linux-arm-ker...@lists.infradead.org Cc: virtualizat...@lists.linux-foundation.org Cc: etna...@lists.freedesktop.org Cc: linux-samsung-...@vger.kernel.org Cc: intel-...@lists.freedesktop.org Cc: linux-media...@lists.infradead.org Cc: linux-amlo...@lists.infradead.org Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: spice-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Cc: linux-renesas-...@vger.kernel.org Cc: linux-rockc...@lists.infradead.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-te...@vger.kernel.org Cc: xen-de...@lists.xen.org --- .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 + .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 2 +- .../display/amdgpu_dm/amdgpu_dm_services.c| 2 +- drivers/gpu/drm/arc/arcpgu_crtc.c | 2 +- drivers/gpu/drm/arc/arcpgu_drv.c | 2 +- drivers/gpu/drm/arc/arcpgu_sim.c | 2 +- drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +- drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- drivers/gpu/drm/arm/malidp_crtc.c | 2 +- drivers/gpu/drm/arm/malidp_drv.c | 2 +- drivers/gpu/drm/arm/malidp_mw.c | 2 +- drivers/gpu/drm/armada/armada_510.c | 2 +- drivers/gpu/drm/armada/armada_crtc.c | 2 +- drivers/gpu/drm/armada/armada_drv.c | 2 +- drivers/gpu/drm/armada/armada_fb.c| 2 +- drivers/gpu/drm/ast/ast_drv.c | 1 + drivers/gpu/drm/ast/ast_mode.c| 1 + .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 2 +- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 +- drivers/gpu/drm/bochs/bochs_drv.c | 1 + drivers/gpu/drm/bochs/bochs_kms.c | 1 + drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 +- .../drm/bridge/analogix/analogix_dp_core.c| 2 +- drivers/gpu/drm/bridge/cdns-dsi.c | 2 +- drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +- .../bridge/megachips-stdp-ge-b850v3-fw.c | 2 +- drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +- drivers/gpu/drm/bridge/panel.c| 2 +- drivers/gpu/drm/bridge/parade-ps8622.c| 2 +- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 2 +- drivers/gpu/drm/bridge/tc358764.c | 2 +- drivers/gpu/drm/bridge/tc358767.c | 2 +- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +- drivers/gpu/drm/bridge/ti-tfp410.c| 2 +- drivers/gpu/drm/cirrus/cirrus_drv.c | 1 + drivers/gpu/drm/cirrus/cirrus_mode.c | 1 + drivers/gpu/drm/drm_atomic_helper.c | 1 - drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- drivers/gpu/drm/drm_modeset_helper.c | 2 +- drivers/gpu/drm/drm_probe_helper.c| 2 +- drivers/gpu/drm/drm_simple_kms_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 - drivers/gpu/drm/exynos/exynos_dp.c| 2 +- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fb.c| 2 +- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c| 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 2 +- drivers/gpu/drm/gma500/psb_intel_drv.h| 1 + .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 +- drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +- .../gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- drivers/gpu/drm/i2c/ch7006_priv.h | 2 +- drivers/gpu/drm/i2c/sil164_drv.c | 2 +- drivers
[Spice-devel] [PATCH 5/7] drm/qxl: Don't set the dpms hook
Doesn't do anything with atomic.
Signed-off-by: Daniel Vetter
Cc: Dave Airlie
Cc: Gerd Hoffmann
Cc: virtualizat...@lists.linux-foundation.org
Cc: spice-devel@lists.freedesktop.org
---
drivers/gpu/drm/qxl/qxl_display.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index ce0b9c40fc21..72a1784dae54 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1010,7 +1010,6 @@ static void qxl_conn_destroy(struct drm_connector
*connector)
}
static const struct drm_connector_funcs qxl_connector_funcs = {
- .dpms = drm_helper_connector_dpms,
.detect = qxl_conn_detect,
.fill_modes = drm_helper_probe_single_connector_modes,
.destroy = qxl_conn_destroy,
--
2.20.0.rc1
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel
[Spice-devel] [PATCH 7/7] drm: Split out drm_probe_helper.h
Having the probe helper stuff (which pretty much everyone needs) in the drm_crtc_helper.h file (which atomic drivers should never need) is confusing. Split them out. To make sure I actually achieved the goal here I went through all drivers. And indeed, all atomic drivers are now free of drm_crtc_helper.h includes. Signed-off-by: Daniel Vetter Cc: linux-arm-ker...@lists.infradead.org Cc: virtualizat...@lists.linux-foundation.org Cc: etna...@lists.freedesktop.org Cc: linux-samsung-...@vger.kernel.org Cc: intel-...@lists.freedesktop.org Cc: linux-media...@lists.infradead.org Cc: linux-amlo...@lists.infradead.org Cc: linux-arm-...@vger.kernel.org Cc: freedr...@lists.freedesktop.org Cc: nouv...@lists.freedesktop.org Cc: spice-devel@lists.freedesktop.org Cc: amd-...@lists.freedesktop.org Cc: linux-renesas-...@vger.kernel.org Cc: linux-rockc...@lists.infradead.org Cc: linux-st...@st-md-mailman.stormreply.com Cc: linux-te...@vger.kernel.org Cc: xen-de...@lists.xen.org --- .../gpu/drm/amd/amdgpu/amdgpu_connectors.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_device.c| 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 2 +- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 1 + .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 2 +- .../amd/display/amdgpu_dm/amdgpu_dm_pp_smu.c | 2 +- .../display/amdgpu_dm/amdgpu_dm_services.c| 2 +- drivers/gpu/drm/arc/arcpgu_crtc.c | 2 +- drivers/gpu/drm/arc/arcpgu_drv.c | 2 +- drivers/gpu/drm/arc/arcpgu_sim.c | 2 +- drivers/gpu/drm/arm/hdlcd_crtc.c | 2 +- drivers/gpu/drm/arm/hdlcd_drv.c | 2 +- drivers/gpu/drm/arm/malidp_crtc.c | 2 +- drivers/gpu/drm/arm/malidp_drv.c | 2 +- drivers/gpu/drm/arm/malidp_mw.c | 2 +- drivers/gpu/drm/armada/armada_510.c | 2 +- drivers/gpu/drm/armada/armada_crtc.c | 2 +- drivers/gpu/drm/armada/armada_drv.c | 2 +- drivers/gpu/drm/armada/armada_fb.c| 2 +- drivers/gpu/drm/ast/ast_drv.c | 1 + drivers/gpu/drm/ast/ast_mode.c| 1 + .../gpu/drm/atmel-hlcdc/atmel_hlcdc_crtc.c| 2 +- drivers/gpu/drm/atmel-hlcdc/atmel_hlcdc_dc.h | 2 +- drivers/gpu/drm/bochs/bochs_drv.c | 1 + drivers/gpu/drm/bochs/bochs_kms.c | 1 + drivers/gpu/drm/bridge/adv7511/adv7511.h | 2 +- drivers/gpu/drm/bridge/analogix-anx78xx.c | 3 +- .../drm/bridge/analogix/analogix_dp_core.c| 2 +- drivers/gpu/drm/bridge/cdns-dsi.c | 2 +- drivers/gpu/drm/bridge/dumb-vga-dac.c | 2 +- .../bridge/megachips-stdp-ge-b850v3-fw.c | 2 +- drivers/gpu/drm/bridge/nxp-ptn3460.c | 2 +- drivers/gpu/drm/bridge/panel.c| 2 +- drivers/gpu/drm/bridge/parade-ps8622.c| 2 +- drivers/gpu/drm/bridge/sii902x.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 2 +- drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c | 2 +- drivers/gpu/drm/bridge/tc358764.c | 2 +- drivers/gpu/drm/bridge/tc358767.c | 2 +- drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +- drivers/gpu/drm/bridge/ti-tfp410.c| 2 +- drivers/gpu/drm/cirrus/cirrus_drv.c | 1 + drivers/gpu/drm/cirrus/cirrus_mode.c | 1 + drivers/gpu/drm/drm_atomic_helper.c | 1 - drivers/gpu/drm/drm_dp_mst_topology.c | 2 +- drivers/gpu/drm/drm_modeset_helper.c | 2 +- drivers/gpu/drm/drm_probe_helper.c| 2 +- drivers/gpu/drm/drm_simple_kms_helper.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_drv.h | 1 - drivers/gpu/drm/exynos/exynos_dp.c| 2 +- drivers/gpu/drm/exynos/exynos_drm_crtc.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_dpi.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_drv.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_dsi.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_fb.c| 2 +- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 2 +- drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_crtc.c| 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_kms.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_plane.c | 2 +- drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_rgb.c | 2 +- drivers/gpu/drm/gma500/psb_intel_drv.h| 1 + .../gpu/drm/hisilicon/hibmc/hibmc_drm_de.c| 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_drv.c | 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_fbdev.c | 2 +- .../gpu/drm/hisilicon/hibmc/hibmc_drm_vdac.c | 2 +- drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 2 +- .../gpu/drm/hisilicon/kirin/kirin_drm_ade.c | 2 +- .../gpu/drm/hisilicon/kirin/kirin_drm_drv.c | 2 +- drivers/gpu/drm/i2c/ch7006_priv.h | 2 +- drivers/gpu/drm/i2c/sil164_drv.c | 2 +- drivers
Re: [Spice-devel] [PATCH] drm/qxl: add spice-devel list to MAINTAINERS
On Wed, Nov 21, 2018 at 10:01:29AM +0100, Gerd Hoffmann wrote: > So qxl kernel patches are sent to the spice-devel list for review. > > Signed-off-by: Gerd Hoffmann Makes sense. Acked-by: Daniel Vetter > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 4caac2f6b0..d9945d57eb 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -4728,6 +4728,7 @@ DRM DRIVER FOR QXL VIRTUAL GPU > M: Dave Airlie > M: Gerd Hoffmann > L: virtualizat...@lists.linux-foundation.org > +L: spice-devel@lists.freedesktop.org > T: git git://anongit.freedesktop.org/drm/drm-misc > S: Maintained > F: drivers/gpu/drm/qxl/ > -- > 2.9.3 > > ___ > dri-devel mailing list > dri-de...@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch ___ Spice-devel mailing list Spice-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/spice-devel
