Re: [PATCH 5/5] KVM: VMX: Always honor guest PAT on CPUs that support self-snoop
> > > Yes? :-) As Gerd described, video memory is "mapped into userspace so > > > the wayland / X11 display server can software-render into the buffer" > > > and it seems that wayland gets something unexpected in this memory and > > > crashes. > > > > Also, I don't know if it helps or not, but out of two hunks in > > 377b2f359d1f, it is the vmx_get_mt_mask() one which brings the > > issue. I.e. the following is enough to fix things: > > > > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > > index f18c2d8c7476..733a0c45d1a6 100644 > > --- a/arch/x86/kvm/vmx/vmx.c > > +++ b/arch/x86/kvm/vmx/vmx.c > > @@ -7659,13 +7659,11 @@ u8 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t > > gfn, bool is_mmio) > > > > /* > > * Force WB and ignore guest PAT if the VM does NOT have a > > non-coherent > > -* device attached and the CPU doesn't support self-snoop. Letting > > the > > -* guest control memory types on Intel CPUs without self-snoop may > > -* result in unexpected behavior, and so KVM's (historical) ABI is > > to > > -* trust the guest to behave only as a last resort. > > +* device attached. Letting the guest control memory types on Intel > > +* CPUs may result in unexpected behavior, and so KVM's ABI is to > > trust > > +* the guest to behave only as a last resort. > > */ > > - if (!static_cpu_has(X86_FEATURE_SELFSNOOP) && > > - !kvm_arch_has_noncoherent_dma(vcpu->kvm)) > > + if (!kvm_arch_has_noncoherent_dma(vcpu->kvm)) > > return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT) | > > VMX_EPT_IPAT_BIT; > > > > return (MTRR_TYPE_WRBACK << VMX_EPT_MT_EPTE_SHIFT); > > Hmm, that suggests the guest kernel maps the buffer as WC. And looking at the > bochs driver, IIUC, the kernel mappings via ioremap() are UC-, not WC. So it > could be that userspace doesn't play nice with WC, but could it also be that > the > QEMU backend doesn't play nice with WC (on Intel)? > > Given that this is a purely synthetic device, is there any reason to use UC > or WC? Well, sharing code with other (real hardware) drivers is pretty much the only reason. DRM has a set of helper functions to manage vram in pci memory bars (see drm_gem_vram_helper.c, drm_gem_ttm_helper.c). > I.e. can the bochs driver configure its VRAM buffers to be WB? It doesn't > look > super easy (the DRM/TTM code has so. many. layers), but it appears doable. > Since > the device only exists in VMs, it's possible the bochs driver has never run on > Intel CPUs with WC memtype. Thomas Zimmermann (Cc'ed) has a drm patch series in flight which switches the bochs driver to a shadow buffer model, i.e. all the buffers visible to fbcon and userspace live in main memory. Display updates are handled via in-kernel memcpy from shadow to vram. The pci memory bar becomes an bochs driver implementation detail not visible outside the driver. This should give the bochs driver the freedom to map vram with whatever attributes work best with kvm, without needing drm changes outside the driver. Of course all this does not help much with current distro kernels broken by this patch ... take care, Gerd
Re: [PATCH] drm/qxl: fix NULL dereference in qxl_add_mode
On Fri, Mar 01, 2024 at 11:55:11AM +0300, Aleksandr Burakov wrote:
> Return value of a function 'drm_cvt_mode' is dereferenced without
> checking for NULL but drm_mode_create() in drm_cvt_mode() may
> return NULL value in case of memory allocation error.
>
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
>
> Fixes: 1b043677d4be ("drm/qxl: add qxl_add_mode helper function")
> Signed-off-by: Aleksandr Burakov
> ---
> drivers/gpu/drm/qxl/qxl_display.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c
> b/drivers/gpu/drm/qxl/qxl_display.c
> index a152a7c6db21..447532c29e02 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -236,8 +236,10 @@ static int qxl_add_mode(struct drm_connector *connector,
> return 0;
>
> mode = drm_cvt_mode(dev, width, height, 60, false, false, false);
> - if (preferred)
> + if (preferred && mode)
> mode->type |= DRM_MODE_TYPE_PREFERRED;
> + else
> + return 0;
> mode->hdisplay = width;
That doesn't fix the NULL pointer dereference in case "preferred" is
false.
I'd suggest "if (!mode) return 0" instead.
Re: [PATCH] qxl: Fix uninitialised struct field head.surface_id
On Thu, Mar 04, 2021 at 09:49:28AM +, Colin King wrote:
> From: Colin Ian King
>
> The surface_id struct field in head is not being initialized and
> static analysis warns that this is being passed through to
> dev->monitors_config->heads[i] on an assignment. Clear up this
> warning by initializing it to zero.
>
> Addresses-Coverity: ("Uninitialized scalar variable")
> Fixes: a6d3c4d79822 ("qxl: hook monitors_config updates into crtc, not
> encoder.")
> Signed-off-by: Colin Ian King
Pushed to drm-misc-fixes.
thanks,
Gerd
Re: drm/ttm: ttm_bo_release called without lock
On Thu, Mar 04, 2021 at 08:42:55AM +0100, Thomas Zimmermann wrote: > (cc'ing Gerd) > > This might be related to the recent clean-up patches for the BO handling in > qxl. Yes, it is. Fixed in drm-misc-next, cherry-picked into drm-misc-fixes, hopefully lands in -rc2. take care, Gerd
Re: [RFC PATCH] drm/vkms: Add support for virtual hardware mode
On Thu, Feb 25, 2021 at 11:32:08AM +0100, Daniel Vetter wrote: > On Thu, Feb 25, 2021 at 11:25:20AM +0100, Gerd Hoffmann wrote: > > On Thu, Feb 25, 2021 at 10:09:42AM +0100, Daniel Vetter wrote: > > > On Wed, Feb 24, 2021 at 11:55 AM Sumera Priyadarsini > > > wrote: > > > > > > > > Add a virtual hardware or vblank-less mode as a module to enable > > > > VKMS to emulate virtual graphic drivers. This mode can be enabled > > > > by setting enable_virtual_hw=1 at the time of loading VKMS. > > > > > > > > A new function vkms_crtc_composer() has been added to bypass the > > > > vblank mode and is called directly in the atomic hook in > > > > vkms_atomic_begin(). However, some crc captures still use vblanks > > > > which causes the crc-based igt tests to crash. Currently, I am unsure > > > > about how to approach one-shot implementation of crc reads so I am > > > > still working on that. > > > > > > Gerd, Zack: For virtual hw like virtio-gpu or vmwgfx that does > > > one-shot upload and damage tracking, what do you think is the best way > > > to capture crc for validation? Assuming that's even on the plans > > > anywhere ... > > > > > > Ideally it'd be a crc that the host side captures, so that we really > > > have end-to-end validation, including the damage uploads and all that. > > > > Disclaimer: Not knowing much about the crc thing beside having noticed > > it exists and seems to be used for display content checking. > > > > > For vkms we're going for now with one-shot crc generation after each > > > atomic flip (or DIRTYFB ioctl call). Will need a pile of igt changes, > > > but seems like the most fitting model. > > > Other option would be that we'd wire up something on the kernel side > > > that generates a crc on-demand every time igt reads a new crc value > > > (maybe with some rate limiting). But that's not really how virtual hw > > > works when everything is pushed explicitly to the host side. > > > > igt runs inside the guest, right? > > Yup. There's some debugfs files for capture crc on a specific CRTC. So > supporting this would mean some virtio-gpu revision so you could ask the > host side for a crc when you do a screen update, and the host side would > send that back to you on a virtio channel as some kind of message. Waded through the source code a bit. So, the vkms crc code merges all planes (specifically the cursor plane) before calculating the crc. Which is a bit of a problem, we try to avoid that and rarely actually merge the planes anywhere in the virtualization stack. Instead we prefer to pass through the cursor plane separately, so we can -- for example -- use that to simply set the cursor sprite of the qemu gtk window. It's much more snappy because moving+rendering the pointer doesn't need a round-trip to the guest then. So, it would be quite some effort on the host side, we would have to merge planes just for crc calculation. > > You can ask qemu to write out a screen dump. Hmm, the (hardware) cursor is not in the screen dump either. A software cursor (when using for example cirrus which has no cursor plane) would be there. > > Another option to access the screen would be vnc. vnc clients can get the cursor sprite. They can't get the position though, only set it (it's a one-way ticket in the vnc protocol). Typically not a problem because desktops set the position in response to the pointer events so guest + host position match nevertheless. But for test cases which don't look at input events and set the cursor to a fixed place this is a problem ... > > On-demand crc via debugfs or ioctl would work too, but yes that wouldn't > > verify the host-side. At least not without virtio protocol extensions. > > We could add a new command asking the host to crc the display and return > > the result for on-demand crc. Or add a crc request flag to an existing > > command (VIRTIO_GPU_CMD_RESOURCE_FLUSH probably). > > Yup, I think that's what would be needed. The question here is, what do > you think would be the most natural way for virtio host side stack to > support this? virtio has feature flags, so we can easily introduce an extension in a backward compatible way. Each command sends a reply, with optional payload, so it would make sense to send the crc with the VIRTIO_GPU_CMD_RESOURCE_FLUSH reply. Alternatively introduce a communication channel independent of the gpu, using for example virtio-serial or vsock, let the guest send crc requests to qemu that way. Which would work with all qemu display devices, not only virtio-gpu. take care, Gerd
Re: [RFC PATCH] drm/vkms: Add support for virtual hardware mode
On Thu, Feb 25, 2021 at 10:09:42AM +0100, Daniel Vetter wrote: > On Wed, Feb 24, 2021 at 11:55 AM Sumera Priyadarsini > wrote: > > > > Add a virtual hardware or vblank-less mode as a module to enable > > VKMS to emulate virtual graphic drivers. This mode can be enabled > > by setting enable_virtual_hw=1 at the time of loading VKMS. > > > > A new function vkms_crtc_composer() has been added to bypass the > > vblank mode and is called directly in the atomic hook in > > vkms_atomic_begin(). However, some crc captures still use vblanks > > which causes the crc-based igt tests to crash. Currently, I am unsure > > about how to approach one-shot implementation of crc reads so I am > > still working on that. > > Gerd, Zack: For virtual hw like virtio-gpu or vmwgfx that does > one-shot upload and damage tracking, what do you think is the best way > to capture crc for validation? Assuming that's even on the plans > anywhere ... > > Ideally it'd be a crc that the host side captures, so that we really > have end-to-end validation, including the damage uploads and all that. Disclaimer: Not knowing much about the crc thing beside having noticed it exists and seems to be used for display content checking. > For vkms we're going for now with one-shot crc generation after each > atomic flip (or DIRTYFB ioctl call). Will need a pile of igt changes, > but seems like the most fitting model. > Other option would be that we'd wire up something on the kernel side > that generates a crc on-demand every time igt reads a new crc value > (maybe with some rate limiting). But that's not really how virtual hw > works when everything is pushed explicitly to the host side. igt runs inside the guest, right? You can ask qemu to write out a screen dump. Reading that and calculate a crc from it should be easy. But the tricky part is how to coordinate guest and host then. qemu autotesting typically runs on the host, connected to qemu (monitor) and guest (ssh or serial console) so it can control both host and guest side. Another option to access the screen would be vnc. With user networking and a guest forwarding rule it should be possible to allow the guest access the qemu vnc server for its own display. Would be more effort to capture the display, but it would for the most part take the host out of the picture. The guest could coordinate everything on its own then. On-demand crc via debugfs or ioctl would work too, but yes that wouldn't verify the host-side. At least not without virtio protocol extensions. We could add a new command asking the host to crc the display and return the result for on-demand crc. Or add a crc request flag to an existing command (VIRTIO_GPU_CMD_RESOURCE_FLUSH probably). take care, Gerd
Re: [PATCH v2 10/11] drm/qxl: rework cursor plane
Hi, > > Well. I suspect I could easily spend a month cleaning up and party > > redesign the qxl driver (specifically qxl_draw.c + qxl_image.c). > > > > I'm not sure I'll find the time to actually do that anytime soon. > > I have plenty of other stuff on my TODO list, and given that the > > world is transitioning to virtio-gpu the priority for qxl isn't > > that high. > > Well, in that case: > > Acked-by: Thomas Zimmermann > > for patches 9 and 10. Having the vmap calls fixed is at least worth it. Thanks. Any chance you can ack 8/11 too (only patch missing an ack). take care, Gerd
Re: [PATCH v2 10/11] drm/qxl: rework cursor plane
Hi, > I'm still trying to wrap my head around the qxl cursor code. > > Getting vmap out of the commit tail is good, but I feel like this isn't > going in the right direction overall. > > In ast, these helper functions were only good when converting the drvier to > atomic modesetting. So I removed them in the latst patchset and did all the > updates in the plane helpers directly. I see the helper functions more as a way to get some structure into the code flow. The callbacks are easier to read if they just call helper functions for stuff which needs more than a handful lines of code (patch 9/11 exists for the same reason). The helpers also make it easier move work from one callback to another, but that is just a useful side-effect. I had considered making that two separate patches, one factor out code into functions and one moving the calls. Turned out to not be that easy though, because the old qxl_cursor_atomic_update() code was a rather hairy mix of qxl_create_cursor() + qxl_primary_apply_cursor() + qxl_primary_move_cursor(). > For cursor_bo itself, it seems to be transitional state that is only used > during the plane update and crtc update . It should probably be stored in a > plane-state structure. > > Some of the primary plane's functions seem to deal with cursor handling. > What's the role of the primary plane in cursor handling? It's a quirk. The qxl device will forget the cursor state on qxl_io_create_primary(), so I have to remember the cursor state and re-establish it by calling qxl_primary_apply_cursor() again. So I'm not sure sticking this into plane state would work. Because of the quirk this is more than just a handover from prepare to commit. > For now, I suggest to merge patch 1 to 8 and 11; and move the cursor patches > into a new patchset. I can merge 1-8, but 11 has to wait until the cursor is sorted. There is a reason why 11 is last in the series ;) > I'd like ot hear Daniel's opinion on this. Do you have > further plans here? Well. I suspect I could easily spend a month cleaning up and party redesign the qxl driver (specifically qxl_draw.c + qxl_image.c). I'm not sure I'll find the time to actually do that anytime soon. I have plenty of other stuff on my TODO list, and given that the world is transitioning to virtio-gpu the priority for qxl isn't that high. So, no, I have no short-term plans for qxl beyond fixing pins + reservations + lockdep. take care, Gerd
[PATCH v2 09/11] drm/qxl: move shadow handling to new qxl_prepare_shadow()
Pure code motion, no functional change.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 61 +--
1 file changed, 34 insertions(+), 27 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index f106da917863..b315d7484e21 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -771,13 +771,45 @@ static void qxl_calc_dumb_shadow(struct qxl_device *qdev,
DRM_DEBUG("%dx%d\n", surf->width, surf->height);
}
+static void qxl_prepare_shadow(struct qxl_device *qdev, struct qxl_bo *user_bo,
+ int crtc_index)
+{
+ struct qxl_surface surf;
+
+ qxl_update_dumb_head(qdev, crtc_index,
+user_bo);
+ qxl_calc_dumb_shadow(qdev, &surf);
+ if (!qdev->dumb_shadow_bo ||
+ qdev->dumb_shadow_bo->surf.width != surf.width ||
+ qdev->dumb_shadow_bo->surf.height != surf.height) {
+ if (qdev->dumb_shadow_bo) {
+ drm_gem_object_put
+ (&qdev->dumb_shadow_bo->tbo.base);
+ qdev->dumb_shadow_bo = NULL;
+ }
+ qxl_bo_create(qdev, surf.height * surf.stride,
+ true, true, QXL_GEM_DOMAIN_SURFACE, 0,
+ &surf, &qdev->dumb_shadow_bo);
+ }
+ if (user_bo->shadow != qdev->dumb_shadow_bo) {
+ if (user_bo->shadow) {
+ qxl_bo_unpin(user_bo->shadow);
+ drm_gem_object_put
+ (&user_bo->shadow->tbo.base);
+ user_bo->shadow = NULL;
+ }
+ drm_gem_object_get(&qdev->dumb_shadow_bo->tbo.base);
+ user_bo->shadow = qdev->dumb_shadow_bo;
+ qxl_bo_pin(user_bo->shadow);
+ }
+}
+
static int qxl_plane_prepare_fb(struct drm_plane *plane,
struct drm_plane_state *new_state)
{
struct qxl_device *qdev = to_qxl(plane->dev);
struct drm_gem_object *obj;
struct qxl_bo *user_bo;
- struct qxl_surface surf;
if (!new_state->fb)
return 0;
@@ -787,32 +819,7 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
if (plane->type == DRM_PLANE_TYPE_PRIMARY &&
user_bo->is_dumb) {
- qxl_update_dumb_head(qdev, new_state->crtc->index,
-user_bo);
- qxl_calc_dumb_shadow(qdev, &surf);
- if (!qdev->dumb_shadow_bo ||
- qdev->dumb_shadow_bo->surf.width != surf.width ||
- qdev->dumb_shadow_bo->surf.height != surf.height) {
- if (qdev->dumb_shadow_bo) {
- drm_gem_object_put
- (&qdev->dumb_shadow_bo->tbo.base);
- qdev->dumb_shadow_bo = NULL;
- }
- qxl_bo_create(qdev, surf.height * surf.stride,
- true, true, QXL_GEM_DOMAIN_SURFACE, 0,
- &surf, &qdev->dumb_shadow_bo);
- }
- if (user_bo->shadow != qdev->dumb_shadow_bo) {
- if (user_bo->shadow) {
- qxl_bo_unpin(user_bo->shadow);
- drm_gem_object_put
- (&user_bo->shadow->tbo.base);
- user_bo->shadow = NULL;
- }
- drm_gem_object_get(&qdev->dumb_shadow_bo->tbo.base);
- user_bo->shadow = qdev->dumb_shadow_bo;
- qxl_bo_pin(user_bo->shadow);
- }
+ qxl_prepare_shadow(qdev, user_bo, new_state->crtc->index);
}
return qxl_bo_pin(user_bo);
--
2.29.2
[PATCH v2 06/11] drm/qxl: add qxl_bo_vmap/qxl_bo_vunmap
Add vmap/vunmap variants which reserve (and pin) the bo.
They can be used in case the caller doesn't hold a reservation
for the bo.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_object.h | 2 ++
drivers/gpu/drm/qxl/qxl_object.c | 36
2 files changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 2495e5cdf353..ee9c29de4d3d 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -64,7 +64,9 @@ extern int qxl_bo_create(struct qxl_device *qdev,
u32 priority,
struct qxl_surface *surf,
struct qxl_bo **bo_ptr);
+int qxl_bo_vmap(struct qxl_bo *bo, struct dma_buf_map *map);
int qxl_bo_vmap_locked(struct qxl_bo *bo, struct dma_buf_map *map);
+int qxl_bo_vunmap(struct qxl_bo *bo);
void qxl_bo_vunmap_locked(struct qxl_bo *bo);
void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, int
page_offset);
void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo,
void *map);
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index f4a015381a7f..82c3bf195ad6 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -29,6 +29,9 @@
#include "qxl_drv.h"
#include "qxl_object.h"
+static int __qxl_bo_pin(struct qxl_bo *bo);
+static void __qxl_bo_unpin(struct qxl_bo *bo);
+
static void qxl_ttm_bo_destroy(struct ttm_buffer_object *tbo)
{
struct qxl_bo *bo;
@@ -179,6 +182,25 @@ int qxl_bo_vmap_locked(struct qxl_bo *bo, struct
dma_buf_map *map)
return 0;
}
+int qxl_bo_vmap(struct qxl_bo *bo, struct dma_buf_map *map)
+{
+ int r;
+
+ r = qxl_bo_reserve(bo);
+ if (r)
+ return r;
+
+ r = __qxl_bo_pin(bo);
+ if (r) {
+ qxl_bo_unreserve(bo);
+ return r;
+ }
+
+ r = qxl_bo_vmap_locked(bo, map);
+ qxl_bo_unreserve(bo);
+ return r;
+}
+
void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev,
struct qxl_bo *bo, int page_offset)
{
@@ -223,6 +245,20 @@ void qxl_bo_vunmap_locked(struct qxl_bo *bo)
ttm_bo_vunmap(&bo->tbo, &bo->map);
}
+int qxl_bo_vunmap(struct qxl_bo *bo)
+{
+ int r;
+
+ r = qxl_bo_reserve(bo);
+ if (r)
+ return r;
+
+ qxl_bo_vunmap_locked(bo);
+ __qxl_bo_unpin(bo);
+ qxl_bo_unreserve(bo);
+ return 0;
+}
+
void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev,
struct qxl_bo *bo, void *pmap)
{
--
2.29.2
[PATCH v2 10/11] drm/qxl: rework cursor plane
Add helper functions to create and move the cursor.
Create the cursor_bo in prepare_fb callback, in the
atomic_commit callback we only send the update command
to the host.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 248 --
1 file changed, 133 insertions(+), 115 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index b315d7484e21..4a3d272e8d6c 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -476,12 +476,11 @@ static int qxl_primary_atomic_check(struct drm_plane
*plane,
return qxl_check_framebuffer(qdev, bo);
}
-static int qxl_primary_apply_cursor(struct drm_plane *plane)
+static int qxl_primary_apply_cursor(struct qxl_device *qdev,
+ struct drm_plane_state *plane_state)
{
- struct drm_device *dev = plane->dev;
- struct qxl_device *qdev = to_qxl(dev);
- struct drm_framebuffer *fb = plane->state->fb;
- struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc);
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct qxl_crtc *qcrtc = to_qxl_crtc(plane_state->crtc);
struct qxl_cursor_cmd *cmd;
struct qxl_release *release;
int ret = 0;
@@ -505,8 +504,8 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane)
cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev, release);
cmd->type = QXL_CURSOR_SET;
- cmd->u.set.position.x = plane->state->crtc_x + fb->hot_x;
- cmd->u.set.position.y = plane->state->crtc_y + fb->hot_y;
+ cmd->u.set.position.x = plane_state->crtc_x + fb->hot_x;
+ cmd->u.set.position.y = plane_state->crtc_y + fb->hot_y;
cmd->u.set.shape = qxl_bo_physical_address(qdev, qcrtc->cursor_bo, 0);
@@ -523,6 +522,113 @@ static int qxl_primary_apply_cursor(struct drm_plane
*plane)
return ret;
}
+static int qxl_primary_move_cursor(struct qxl_device *qdev,
+ struct drm_plane_state *plane_state)
+{
+ struct drm_framebuffer *fb = plane_state->fb;
+ struct qxl_crtc *qcrtc = to_qxl_crtc(plane_state->crtc);
+ struct qxl_cursor_cmd *cmd;
+ struct qxl_release *release;
+ int ret = 0;
+
+ if (!qcrtc->cursor_bo)
+ return 0;
+
+ ret = qxl_alloc_release_reserved(qdev, sizeof(*cmd),
+QXL_RELEASE_CURSOR_CMD,
+&release, NULL);
+ if (ret)
+ return ret;
+
+ ret = qxl_release_reserve_list(release, true);
+ if (ret) {
+ qxl_release_free(qdev, release);
+ return ret;
+ }
+
+ cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev, release);
+ cmd->type = QXL_CURSOR_MOVE;
+ cmd->u.position.x = plane_state->crtc_x + fb->hot_x;
+ cmd->u.position.y = plane_state->crtc_y + fb->hot_y;
+ qxl_release_unmap(qdev, release, &cmd->release_info);
+
+ qxl_release_fence_buffer_objects(release);
+ qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
+ return ret;
+}
+
+static struct qxl_bo *qxl_create_cursor(struct qxl_device *qdev,
+ struct qxl_bo *user_bo,
+ int hot_x, int hot_y)
+{
+ static const u32 size = 64 * 64 * 4;
+ struct qxl_bo *cursor_bo;
+ struct dma_buf_map cursor_map;
+ struct dma_buf_map user_map;
+ struct qxl_cursor cursor;
+ int ret;
+
+ if (!user_bo)
+ return NULL;
+
+ ret = qxl_bo_create(qdev, sizeof(struct qxl_cursor) + size,
+ false, true, QXL_GEM_DOMAIN_VRAM, 1,
+ NULL, &cursor_bo);
+ if (ret)
+ goto err;
+
+ ret = qxl_bo_vmap(cursor_bo, &cursor_map);
+ if (ret)
+ goto err_unref;
+
+ ret = qxl_bo_vmap(user_bo, &user_map);
+ if (ret)
+ goto err_unmap;
+
+ cursor.header.unique = 0;
+ cursor.header.type = SPICE_CURSOR_TYPE_ALPHA;
+ cursor.header.width = 64;
+ cursor.header.height = 64;
+ cursor.header.hot_spot_x = hot_x;
+ cursor.header.hot_spot_y = hot_y;
+ cursor.data_size = size;
+ cursor.chunk.next_chunk = 0;
+ cursor.chunk.prev_chunk = 0;
+ cursor.chunk.data_size = size;
+ if (cursor_map.is_iomem) {
+ memcpy_toio(cursor_map.vaddr_iomem,
+ &cursor, sizeof(cursor));
+ memcpy_toio(cursor_map.vaddr_iomem + sizeof(cursor),
+ user_map.vaddr, size);
+ } else {
+ memcpy(cursor_map.vaddr,
+ &cursor, sizeof(cursor));
+
[PATCH v2 07/11] drm/qxl: fix prime vmap
Use the correct vmap variant. We don't have a reservation here,
so we can't use the _locked version.
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_prime.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
index 2bebe662516f..0628d1cc91fe 100644
--- a/drivers/gpu/drm/qxl/qxl_prime.c
+++ b/drivers/gpu/drm/qxl/qxl_prime.c
@@ -59,7 +59,7 @@ int qxl_gem_prime_vmap(struct drm_gem_object *obj, struct
dma_buf_map *map)
struct qxl_bo *bo = gem_to_qxl_bo(obj);
int ret;
- ret = qxl_bo_vmap_locked(bo, map);
+ ret = qxl_bo_vmap(bo, map);
if (ret < 0)
return ret;
@@ -71,7 +71,7 @@ void qxl_gem_prime_vunmap(struct drm_gem_object *obj,
{
struct qxl_bo *bo = gem_to_qxl_bo(obj);
- qxl_bo_vunmap_locked(bo);
+ qxl_bo_vunmap(bo);
}
int qxl_gem_prime_mmap(struct drm_gem_object *obj,
--
2.29.2
[PATCH v2 08/11] drm/qxl: fix monitors object vmap
Use the correct vmap variant. We don't hold a reservation here, so we can't use the _locked variant. We can drop the pin because qxl_bo_vmap will do that for us. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_display.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index bfcc93089a94..f106da917863 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -1159,12 +1159,10 @@ int qxl_create_monitors_object(struct qxl_device *qdev) } qdev->monitors_config_bo = gem_to_qxl_bo(gobj); - ret = qxl_bo_pin(qdev->monitors_config_bo); + ret = qxl_bo_vmap(qdev->monitors_config_bo, &map); if (ret) return ret; - qxl_bo_vmap_locked(qdev->monitors_config_bo, &map); - qdev->monitors_config = qdev->monitors_config_bo->kptr; qdev->ram_header->monitors_config = qxl_bo_physical_address(qdev, qdev->monitors_config_bo, 0); @@ -1189,8 +1187,7 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev) qdev->monitors_config = NULL; qdev->ram_header->monitors_config = 0; - qxl_bo_vunmap_locked(qdev->monitors_config_bo); - ret = qxl_bo_unpin(qdev->monitors_config_bo); + ret = qxl_bo_vunmap(qdev->monitors_config_bo); if (ret) return ret; -- 2.29.2
[PATCH v2 11/11] drm/qxl: add lock asserts to qxl_bo_vmap_locked + qxl_bo_vunmap_locked
Try avoid re-introducing locking bugs.
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_object.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 82c3bf195ad6..6e26d70f2f07 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -162,6 +162,8 @@ int qxl_bo_vmap_locked(struct qxl_bo *bo, struct
dma_buf_map *map)
{
int r;
+ dma_resv_assert_held(bo->tbo.base.resv);
+
if (bo->kptr) {
bo->map_count++;
goto out;
@@ -236,6 +238,8 @@ void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev,
void qxl_bo_vunmap_locked(struct qxl_bo *bo)
{
+ dma_resv_assert_held(bo->tbo.base.resv);
+
if (bo->kptr == NULL)
return;
bo->map_count--;
--
2.29.2
[PATCH v2 03/11] drm/qxl: use ttm bo priorities
Allow to set priorities for buffer objects. Use priority 1 for surface
and cursor command releases. Use priority 0 for drawing command
releases. That way the short-living drawing commands are first in line
when it comes to eviction, making it *much* less likely that
ttm_bo_mem_force_space() picks something which can't be evicted and
throws an error after waiting a while without success.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_object.h | 1 +
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
drivers/gpu/drm/qxl/qxl_gem.c | 2 +-
drivers/gpu/drm/qxl/qxl_object.c | 5 +++--
drivers/gpu/drm/qxl/qxl_release.c | 18 --
6 files changed, 20 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index e60a8f88e226..dc1659e717f1 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -61,6 +61,7 @@ static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
extern int qxl_bo_create(struct qxl_device *qdev,
unsigned long size,
bool kernel, bool pinned, u32 domain,
+u32 priority,
struct qxl_surface *surf,
struct qxl_bo **bo_ptr);
extern int qxl_bo_kmap(struct qxl_bo *bo, struct dma_buf_map *map);
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 7e22a81bfb36..7b00c955cd82 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -269,7 +269,7 @@ int qxl_alloc_bo_reserved(struct qxl_device *qdev,
int ret;
ret = qxl_bo_create(qdev, size, false /* not kernel - device */,
- false, QXL_GEM_DOMAIN_VRAM, NULL, &bo);
+ false, QXL_GEM_DOMAIN_VRAM, 0, NULL, &bo);
if (ret) {
DRM_ERROR("failed to allocate VRAM BO\n");
return ret;
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index ec50d2cfd4e1..a1b5cc5918bc 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -799,8 +799,8 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
qdev->dumb_shadow_bo = NULL;
}
qxl_bo_create(qdev, surf.height * surf.stride,
- true, true, QXL_GEM_DOMAIN_SURFACE, &surf,
- &qdev->dumb_shadow_bo);
+ true, true, QXL_GEM_DOMAIN_SURFACE, 0,
+ &surf, &qdev->dumb_shadow_bo);
}
if (user_bo->shadow != qdev->dumb_shadow_bo) {
if (user_bo->shadow) {
diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c
index 48e096285b4c..a08da0bd9098 100644
--- a/drivers/gpu/drm/qxl/qxl_gem.c
+++ b/drivers/gpu/drm/qxl/qxl_gem.c
@@ -55,7 +55,7 @@ int qxl_gem_object_create(struct qxl_device *qdev, int size,
/* At least align on page size */
if (alignment < PAGE_SIZE)
alignment = PAGE_SIZE;
- r = qxl_bo_create(qdev, size, kernel, false, initial_domain, surf,
&qbo);
+ r = qxl_bo_create(qdev, size, kernel, false, initial_domain, 0, surf,
&qbo);
if (r) {
if (r != -ERESTARTSYS)
DRM_ERROR(
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 705b51535492..7eada4ad217b 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -103,8 +103,8 @@ static const struct drm_gem_object_funcs qxl_object_funcs =
{
.print_info = drm_gem_ttm_print_info,
};
-int qxl_bo_create(struct qxl_device *qdev,
- unsigned long size, bool kernel, bool pinned, u32 domain,
+int qxl_bo_create(struct qxl_device *qdev, unsigned long size,
+ bool kernel, bool pinned, u32 domain, u32 priority,
struct qxl_surface *surf,
struct qxl_bo **bo_ptr)
{
@@ -137,6 +137,7 @@ int qxl_bo_create(struct qxl_device *qdev,
qxl_ttm_placement_from_domain(bo, domain);
+ bo->tbo.priority = priority;
r = ttm_bo_init_reserved(&qdev->mman.bdev, &bo->tbo, size, type,
&bo->placement, 0, &ctx, NULL, NULL,
&qxl_ttm_bo_destroy);
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 579c6de10c8e..716d706ca7f0 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -160,11 +160,12 @@ qxl_release_free(struct qxl_device *qdev,
}
static int qxl_release_bo_alloc(struct qxl_device *qdev,
- struct
[PATCH v2 05/11] drm/qxl: rename qxl_bo_kmap -> qxl_bo_vmap_locked
Append _locked to Make clear that these functions should be called with reserved bo's only. While being at it also rename kmap -> vmap. No functional change. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_object.h | 4 ++-- drivers/gpu/drm/qxl/qxl_display.c | 14 +++--- drivers/gpu/drm/qxl/qxl_draw.c| 8 drivers/gpu/drm/qxl/qxl_image.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 8 drivers/gpu/drm/qxl/qxl_prime.c | 4 ++-- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index dc1659e717f1..2495e5cdf353 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -64,8 +64,8 @@ extern int qxl_bo_create(struct qxl_device *qdev, u32 priority, struct qxl_surface *surf, struct qxl_bo **bo_ptr); -extern int qxl_bo_kmap(struct qxl_bo *bo, struct dma_buf_map *map); -extern void qxl_bo_kunmap(struct qxl_bo *bo); +int qxl_bo_vmap_locked(struct qxl_bo *bo, struct dma_buf_map *map); +void qxl_bo_vunmap_locked(struct qxl_bo *bo); void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, int page_offset); void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, void *map); extern struct qxl_bo *qxl_bo_ref(struct qxl_bo *bo); diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index a1b5cc5918bc..bfcc93089a94 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -600,7 +600,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, user_bo = gem_to_qxl_bo(obj); /* pinning is done in the prepare/cleanup framevbuffer */ - ret = qxl_bo_kmap(user_bo, &user_map); + ret = qxl_bo_vmap_locked(user_bo, &user_map); if (ret) goto out_free_release; user_ptr = user_map.vaddr; /* TODO: Use mapping abstraction properly */ @@ -619,7 +619,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, if (ret) goto out_unpin; - ret = qxl_bo_kmap(cursor_bo, &cursor_map); + ret = qxl_bo_vmap_locked(cursor_bo, &cursor_map); if (ret) goto out_backoff; if (cursor_map.is_iomem) /* TODO: Use mapping abstraction properly */ @@ -638,8 +638,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, cursor->chunk.prev_chunk = 0; cursor->chunk.data_size = size; memcpy(cursor->chunk.data, user_ptr, size); - qxl_bo_kunmap(cursor_bo); - qxl_bo_kunmap(user_bo); + qxl_bo_vunmap_locked(cursor_bo); + qxl_bo_vunmap_locked(user_bo); cmd = (struct qxl_cursor_cmd *) qxl_release_map(qdev, release); cmd->u.set.visible = 1; @@ -681,7 +681,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, out_free_bo: qxl_bo_unref(&cursor_bo); out_kunmap: - qxl_bo_kunmap(user_bo); + qxl_bo_vunmap_locked(user_bo); out_free_release: qxl_release_free(qdev, release); return; @@ -1163,7 +1163,7 @@ int qxl_create_monitors_object(struct qxl_device *qdev) if (ret) return ret; - qxl_bo_kmap(qdev->monitors_config_bo, &map); + qxl_bo_vmap_locked(qdev->monitors_config_bo, &map); qdev->monitors_config = qdev->monitors_config_bo->kptr; qdev->ram_header->monitors_config = @@ -1189,7 +1189,7 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev) qdev->monitors_config = NULL; qdev->ram_header->monitors_config = 0; - qxl_bo_kunmap(qdev->monitors_config_bo); + qxl_bo_vunmap_locked(qdev->monitors_config_bo); ret = qxl_bo_unpin(qdev->monitors_config_bo); if (ret) return ret; diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c index 7b7acb910780..7d27891e87fa 100644 --- a/drivers/gpu/drm/qxl/qxl_draw.c +++ b/drivers/gpu/drm/qxl/qxl_draw.c @@ -48,7 +48,7 @@ static struct qxl_rect *drawable_set_clipping(struct qxl_device *qdev, struct qxl_clip_rects *dev_clips; int ret; - ret = qxl_bo_kmap(clips_bo, &map); + ret = qxl_bo_vmap_locked(clips_bo, &map); if (ret) return NULL; dev_clips = map.vaddr; /* TODO: Use mapping abstraction properly */ @@ -202,7 +202,7 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev, if (ret) goto out_release_backoff; - ret = qxl_bo_kmap(bo, &surface_map); + ret = qxl_bo_vmap_locked(bo, &surface_map); if (ret) goto ou
[PATCH v2 04/11] drm/qxl: fix lockdep issue in qxl_alloc_release_reserved
Call qxl_bo_unpin (which does a reservation) without holding the
release_mutex lock. Fixes lockdep (correctly) warning on a possible
deadlock.
Fixes: 65ffea3c6e73 ("drm/qxl: unpin release objects")
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_release.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 716d706ca7f0..f5845c96d414 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -283,7 +283,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
int type, struct qxl_release **release,
struct qxl_bo **rbo)
{
- struct qxl_bo *bo;
+ struct qxl_bo *bo, *free_bo = NULL;
int idr_ret;
int ret = 0;
union qxl_release_info *info;
@@ -315,8 +315,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
- qxl_bo_unpin(qdev->current_release_bo[cur_idx]);
- qxl_bo_unref(&qdev->current_release_bo[cur_idx]);
+ free_bo = qdev->current_release_bo[cur_idx];
qdev->current_release_bo_offset[cur_idx] = 0;
qdev->current_release_bo[cur_idx] = NULL;
}
@@ -324,6 +323,10 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
ret = qxl_release_bo_alloc(qdev,
&qdev->current_release_bo[cur_idx], priority);
if (ret) {
mutex_unlock(&qdev->release_mutex);
+ if (free_bo) {
+ qxl_bo_unpin(free_bo);
+ qxl_bo_unref(&free_bo);
+ }
qxl_release_free(qdev, *release);
return ret;
}
@@ -339,6 +342,10 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
*rbo = bo;
mutex_unlock(&qdev->release_mutex);
+ if (free_bo) {
+ qxl_bo_unpin(free_bo);
+ qxl_bo_unref(&free_bo);
+ }
ret = qxl_release_list_add(*release, bo);
qxl_bo_unref(&bo);
--
2.29.2
[PATCH v2 01/11] drm/qxl: properly handle device init failures
Specifically do not try release resources which where
not allocated in the first place.
Cc: Tong Zhang
Tested-by: Tong Zhang
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_display.c | 3 +++
drivers/gpu/drm/qxl/qxl_kms.c | 4
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index c326412136c5..ec50d2cfd4e1 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1183,6 +1183,9 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev)
{
int ret;
+ if (!qdev->monitors_config_bo)
+ return 0;
+
qdev->monitors_config = NULL;
qdev->ram_header->monitors_config = 0;
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 66d74aaaee06..4dc5ad13f12c 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -288,6 +288,10 @@ void qxl_device_fini(struct qxl_device *qdev)
{
int cur_idx;
+ /* check if qxl_device_init() was successful (gc_work is initialized
last) */
+ if (!qdev->gc_work.func)
+ return;
+
for (cur_idx = 0; cur_idx < 3; cur_idx++) {
if (!qdev->current_release_bo[cur_idx])
continue;
--
2.29.2
[PATCH v2 02/11] drm/qxl: more fence wait rework
Move qxl_io_notify_oom() call into wait condition.
That way the driver will call it again if one call
wasn't enough.
Also allows to remove the extra dma_fence_is_signaled()
check and the goto.
Fixes: 5a838e5d5825 ("drm/qxl: simplify qxl_fence_wait")
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_release.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 6ed673d75f9f..579c6de10c8e 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -62,16 +62,12 @@ static long qxl_fence_wait(struct dma_fence *fence, bool
intr,
qdev = container_of(fence->lock, struct qxl_device, release_lock);
- if (dma_fence_is_signaled(fence))
- goto signaled;
-
- qxl_io_notify_oom(qdev);
if (!wait_event_timeout(qdev->release_event,
- dma_fence_is_signaled(fence),
+ (dma_fence_is_signaled(fence) ||
+(qxl_io_notify_oom(qdev), 0)),
timeout))
return 0;
-signaled:
cur = jiffies;
if (time_after(cur, end))
return 0;
--
2.29.2
Re: [PATCH 09/10] drm/qxl: map/unmap framebuffers in prepare_fb+cleanup_fb callbacks.
On Tue, Feb 16, 2021 at 02:46:21PM +0100, Thomas Zimmermann wrote: > > > Am 16.02.21 um 14:27 schrieb Thomas Zimmermann: > > Hi > > > > this is a shadow-buffered plane. Did you consider using the new helpers > > for shadow-buffered planes? They will map the user BO for you and > > provide the mapping in the plane state. > > > > From there, you should implement your own plane state on top of struct > > drm_shadow_plane_state, and also move all the other allocations and > > vmaps into prepare_fb and cleanup_fb. Most of this is not actually > > allowed in commit tails. All we'd have to do is to export the reset, > > duplicate and destroy code; similar to what > > __drm_atomic_helper_plane_reset() does. > > AFAICT the cursor_bo is used to implement double buffering for the cursor > image. > > Ideally, you can do what ast does: pre-allocate/vmap 2 BOs at the end of the > vram. Then pageflip between them in atomic_update(). Resolves all the > allocation and mapping headaches. Just waded through the ast patches. It is not that simple for qxl. You have to send a command to the virtualization host and take care of the host accessing that memory when processing the command, so you can't reuse the memory until the host signals it is fine to do so. But, yes, it should be possible to handle cursor_bo creation in prepare_fb without too much effort. take care, Gerd
[PATCH 08/10] drm/qxl: fix monitors object kmap
Use the correct kmap variant. We don't hold a reservation here, so we can't use the _locked variant. We can drop the pin because qxl_bo_kmap will do that for us. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_display.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index 8672385a1caf..7500560db8e4 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -1159,12 +1159,10 @@ int qxl_create_monitors_object(struct qxl_device *qdev) } qdev->monitors_config_bo = gem_to_qxl_bo(gobj); - ret = qxl_bo_pin(qdev->monitors_config_bo); + ret = qxl_bo_kmap(qdev->monitors_config_bo, &map); if (ret) return ret; - qxl_bo_kmap_locked(qdev->monitors_config_bo, &map); - qdev->monitors_config = qdev->monitors_config_bo->kptr; qdev->ram_header->monitors_config = qxl_bo_physical_address(qdev, qdev->monitors_config_bo, 0); @@ -1189,8 +1187,7 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev) qdev->monitors_config = NULL; qdev->ram_header->monitors_config = 0; - qxl_bo_kunmap_locked(qdev->monitors_config_bo); - ret = qxl_bo_unpin(qdev->monitors_config_bo); + ret = qxl_bo_kunmap(qdev->monitors_config_bo); if (ret) return ret; -- 2.29.2
[PATCH 09/10] drm/qxl: map/unmap framebuffers in prepare_fb+cleanup_fb callbacks.
We don't have to map in atomic_update callback then,
making locking a bit less complicated.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 14 +-
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 7500560db8e4..39b8c5116d34 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -584,7 +584,6 @@ static void qxl_cursor_atomic_update(struct drm_plane
*plane,
struct drm_gem_object *obj;
struct qxl_bo *cursor_bo = NULL, *user_bo = NULL, *old_cursor_bo = NULL;
int ret;
- struct dma_buf_map user_map;
struct dma_buf_map cursor_map;
void *user_ptr;
int size = 64*64*4;
@@ -599,11 +598,8 @@ static void qxl_cursor_atomic_update(struct drm_plane
*plane,
obj = fb->obj[0];
user_bo = gem_to_qxl_bo(obj);
- /* pinning is done in the prepare/cleanup framevbuffer */
- ret = qxl_bo_kmap_locked(user_bo, &user_map);
- if (ret)
- goto out_free_release;
- user_ptr = user_map.vaddr; /* TODO: Use mapping abstraction
properly */
+ /* mapping is done in the prepare/cleanup framevbuffer */
+ user_ptr = user_bo->map.vaddr; /* TODO: Use mapping abstraction
properly */
ret = qxl_alloc_bo_reserved(qdev, release,
sizeof(struct qxl_cursor) + size,
@@ -639,7 +635,6 @@ static void qxl_cursor_atomic_update(struct drm_plane
*plane,
cursor->chunk.data_size = size;
memcpy(cursor->chunk.data, user_ptr, size);
qxl_bo_kunmap_locked(cursor_bo);
- qxl_bo_kunmap_locked(user_bo);
cmd = (struct qxl_cursor_cmd *) qxl_release_map(qdev, release);
cmd->u.set.visible = 1;
@@ -778,6 +773,7 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
struct drm_gem_object *obj;
struct qxl_bo *user_bo;
struct qxl_surface surf;
+ struct dma_buf_map unused;
if (!new_state->fb)
return 0;
@@ -815,7 +811,7 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
}
}
- return qxl_bo_pin(user_bo);
+ return qxl_bo_kmap(user_bo, &unused);
}
static void qxl_plane_cleanup_fb(struct drm_plane *plane,
@@ -834,7 +830,7 @@ static void qxl_plane_cleanup_fb(struct drm_plane *plane,
obj = old_state->fb->obj[0];
user_bo = gem_to_qxl_bo(obj);
- qxl_bo_unpin(user_bo);
+ qxl_bo_kunmap(user_bo);
if (old_state->fb != plane->state->fb && user_bo->shadow) {
qxl_bo_unpin(user_bo->shadow);
--
2.29.2
[PATCH 05/10] drm/qxl: rename qxl_bo_kmap -> qxl_bo_kmap_locked
Make clear that these functions should be called with reserved bo's only. No functional change. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_object.h | 4 ++-- drivers/gpu/drm/qxl/qxl_display.c | 14 +++--- drivers/gpu/drm/qxl/qxl_draw.c| 8 drivers/gpu/drm/qxl/qxl_image.c | 2 +- drivers/gpu/drm/qxl/qxl_object.c | 8 drivers/gpu/drm/qxl/qxl_prime.c | 4 ++-- 6 files changed, 20 insertions(+), 20 deletions(-) diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h index dc1659e717f1..5bd33650183f 100644 --- a/drivers/gpu/drm/qxl/qxl_object.h +++ b/drivers/gpu/drm/qxl/qxl_object.h @@ -64,8 +64,8 @@ extern int qxl_bo_create(struct qxl_device *qdev, u32 priority, struct qxl_surface *surf, struct qxl_bo **bo_ptr); -extern int qxl_bo_kmap(struct qxl_bo *bo, struct dma_buf_map *map); -extern void qxl_bo_kunmap(struct qxl_bo *bo); +extern int qxl_bo_kmap_locked(struct qxl_bo *bo, struct dma_buf_map *map); +extern void qxl_bo_kunmap_locked(struct qxl_bo *bo); void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, int page_offset); void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, void *map); extern struct qxl_bo *qxl_bo_ref(struct qxl_bo *bo); diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index a1b5cc5918bc..8672385a1caf 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -600,7 +600,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, user_bo = gem_to_qxl_bo(obj); /* pinning is done in the prepare/cleanup framevbuffer */ - ret = qxl_bo_kmap(user_bo, &user_map); + ret = qxl_bo_kmap_locked(user_bo, &user_map); if (ret) goto out_free_release; user_ptr = user_map.vaddr; /* TODO: Use mapping abstraction properly */ @@ -619,7 +619,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, if (ret) goto out_unpin; - ret = qxl_bo_kmap(cursor_bo, &cursor_map); + ret = qxl_bo_kmap_locked(cursor_bo, &cursor_map); if (ret) goto out_backoff; if (cursor_map.is_iomem) /* TODO: Use mapping abstraction properly */ @@ -638,8 +638,8 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, cursor->chunk.prev_chunk = 0; cursor->chunk.data_size = size; memcpy(cursor->chunk.data, user_ptr, size); - qxl_bo_kunmap(cursor_bo); - qxl_bo_kunmap(user_bo); + qxl_bo_kunmap_locked(cursor_bo); + qxl_bo_kunmap_locked(user_bo); cmd = (struct qxl_cursor_cmd *) qxl_release_map(qdev, release); cmd->u.set.visible = 1; @@ -681,7 +681,7 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane, out_free_bo: qxl_bo_unref(&cursor_bo); out_kunmap: - qxl_bo_kunmap(user_bo); + qxl_bo_kunmap_locked(user_bo); out_free_release: qxl_release_free(qdev, release); return; @@ -1163,7 +1163,7 @@ int qxl_create_monitors_object(struct qxl_device *qdev) if (ret) return ret; - qxl_bo_kmap(qdev->monitors_config_bo, &map); + qxl_bo_kmap_locked(qdev->monitors_config_bo, &map); qdev->monitors_config = qdev->monitors_config_bo->kptr; qdev->ram_header->monitors_config = @@ -1189,7 +1189,7 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev) qdev->monitors_config = NULL; qdev->ram_header->monitors_config = 0; - qxl_bo_kunmap(qdev->monitors_config_bo); + qxl_bo_kunmap_locked(qdev->monitors_config_bo); ret = qxl_bo_unpin(qdev->monitors_config_bo); if (ret) return ret; diff --git a/drivers/gpu/drm/qxl/qxl_draw.c b/drivers/gpu/drm/qxl/qxl_draw.c index 7b7acb910780..294542d49015 100644 --- a/drivers/gpu/drm/qxl/qxl_draw.c +++ b/drivers/gpu/drm/qxl/qxl_draw.c @@ -48,7 +48,7 @@ static struct qxl_rect *drawable_set_clipping(struct qxl_device *qdev, struct qxl_clip_rects *dev_clips; int ret; - ret = qxl_bo_kmap(clips_bo, &map); + ret = qxl_bo_kmap_locked(clips_bo, &map); if (ret) return NULL; dev_clips = map.vaddr; /* TODO: Use mapping abstraction properly */ @@ -202,7 +202,7 @@ void qxl_draw_dirty_fb(struct qxl_device *qdev, if (ret) goto out_release_backoff; - ret = qxl_bo_kmap(bo, &surface_map); + ret = qxl_bo_kmap_locked(bo, &surface_map); if (ret) goto out_release_backoff; surface_base = surface
[PATCH 06/10] drm/qxl: add qxl_bo_kmap/qxl_bo_kunmap
Add kmap/kunmap variants which reserve (and pin) the bo.
They can be used in case the caller doesn't hold a reservation
for the bo.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_object.h | 2 ++
drivers/gpu/drm/qxl/qxl_object.c | 36
2 files changed, 38 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index 5bd33650183f..360972ae4869 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -64,7 +64,9 @@ extern int qxl_bo_create(struct qxl_device *qdev,
u32 priority,
struct qxl_surface *surf,
struct qxl_bo **bo_ptr);
+extern int qxl_bo_kmap(struct qxl_bo *bo, struct dma_buf_map *map);
extern int qxl_bo_kmap_locked(struct qxl_bo *bo, struct dma_buf_map *map);
+extern int qxl_bo_kunmap(struct qxl_bo *bo);
extern void qxl_bo_kunmap_locked(struct qxl_bo *bo);
void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo, int
page_offset);
void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev, struct qxl_bo *bo,
void *map);
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index b56d4dfc28cb..22748b9566af 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -29,6 +29,9 @@
#include "qxl_drv.h"
#include "qxl_object.h"
+static int __qxl_bo_pin(struct qxl_bo *bo);
+static void __qxl_bo_unpin(struct qxl_bo *bo);
+
static void qxl_ttm_bo_destroy(struct ttm_buffer_object *tbo)
{
struct qxl_bo *bo;
@@ -179,6 +182,25 @@ int qxl_bo_kmap_locked(struct qxl_bo *bo, struct
dma_buf_map *map)
return 0;
}
+int qxl_bo_kmap(struct qxl_bo *bo, struct dma_buf_map *map)
+{
+ int r;
+
+ r = qxl_bo_reserve(bo);
+ if (r)
+ return r;
+
+ r = __qxl_bo_pin(bo);
+ if (r) {
+ qxl_bo_unreserve(bo);
+ return r;
+ }
+
+ r = qxl_bo_kmap_locked(bo, map);
+ qxl_bo_unreserve(bo);
+ return r;
+}
+
void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev,
struct qxl_bo *bo, int page_offset)
{
@@ -223,6 +245,20 @@ void qxl_bo_kunmap_locked(struct qxl_bo *bo)
ttm_bo_vunmap(&bo->tbo, &bo->map);
}
+int qxl_bo_kunmap(struct qxl_bo *bo)
+{
+ int r;
+
+ r = qxl_bo_reserve(bo);
+ if (r)
+ return r;
+
+ qxl_bo_kunmap_locked(bo);
+ __qxl_bo_unpin(bo);
+ qxl_bo_unreserve(bo);
+ return 0;
+}
+
void qxl_bo_kunmap_atomic_page(struct qxl_device *qdev,
struct qxl_bo *bo, void *pmap)
{
--
2.29.2
[PATCH 04/10] drm/qxl: fix lockdep issue in qxl_alloc_release_reserved
Call qxl_bo_unpin (which does a reservation) without holding the
release_mutex lock. Fixes lockdep (correctly) warning on a possible
deadlock.
Fixes: 65ffea3c6e73 ("drm/qxl: unpin release objects")
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_release.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index a831184e014a..352a11a8485b 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -284,7 +284,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
int type, struct qxl_release **release,
struct qxl_bo **rbo)
{
- struct qxl_bo *bo;
+ struct qxl_bo *bo, *free_bo = NULL;
int idr_ret;
int ret = 0;
union qxl_release_info *info;
@@ -317,8 +317,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
- qxl_bo_unpin(qdev->current_release_bo[cur_idx]);
- qxl_bo_unref(&qdev->current_release_bo[cur_idx]);
+ free_bo = qdev->current_release_bo[cur_idx];
qdev->current_release_bo_offset[cur_idx] = 0;
qdev->current_release_bo[cur_idx] = NULL;
}
@@ -326,6 +325,10 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
ret = qxl_release_bo_alloc(qdev,
&qdev->current_release_bo[cur_idx], priority);
if (ret) {
mutex_unlock(&qdev->release_mutex);
+ if (free_bo) {
+ qxl_bo_unpin(free_bo);
+ qxl_bo_unref(&free_bo);
+ }
qxl_release_free(qdev, *release);
return ret;
}
@@ -341,6 +344,10 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
*rbo = bo;
mutex_unlock(&qdev->release_mutex);
+ if (free_bo) {
+ qxl_bo_unpin(free_bo);
+ qxl_bo_unref(&free_bo);
+ }
ret = qxl_release_list_add(*release, bo);
qxl_bo_unref(&bo);
--
2.29.2
[PATCH 10/10] drm/qxl: add lock asserts to qxl_bo_kmap_locked + qxl_bo_kunmap_locked
Try avoid re-introducing locking bugs.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_object.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 22748b9566af..90d5e5b7f927 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -162,6 +162,8 @@ int qxl_bo_kmap_locked(struct qxl_bo *bo, struct
dma_buf_map *map)
{
int r;
+ dma_resv_assert_held(bo->tbo.base.resv);
+
if (bo->kptr) {
bo->map_count++;
goto out;
@@ -236,6 +238,8 @@ void *qxl_bo_kmap_atomic_page(struct qxl_device *qdev,
void qxl_bo_kunmap_locked(struct qxl_bo *bo)
{
+ dma_resv_assert_held(bo->tbo.base.resv);
+
if (bo->kptr == NULL)
return;
bo->map_count--;
--
2.29.2
[PATCH 01/10] drm/qxl: properly handle device init failures
Specifically do not try release resources which where
not allocated in the first place.
Cc: Tong Zhang
Tested-by: Tong Zhang
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 3 +++
drivers/gpu/drm/qxl/qxl_kms.c | 4
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index c326412136c5..ec50d2cfd4e1 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1183,6 +1183,9 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev)
{
int ret;
+ if (!qdev->monitors_config_bo)
+ return 0;
+
qdev->monitors_config = NULL;
qdev->ram_header->monitors_config = 0;
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 66d74aaaee06..4dc5ad13f12c 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -288,6 +288,10 @@ void qxl_device_fini(struct qxl_device *qdev)
{
int cur_idx;
+ /* check if qxl_device_init() was successful (gc_work is initialized
last) */
+ if (!qdev->gc_work.func)
+ return;
+
for (cur_idx = 0; cur_idx < 3; cur_idx++) {
if (!qdev->current_release_bo[cur_idx])
continue;
--
2.29.2
[PATCH 07/10] drm/qxl: fix prime kmap
Use the correct kmap variant. We don't have a reservation here,
so we can't use the _locked version.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_prime.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_prime.c b/drivers/gpu/drm/qxl/qxl_prime.c
index dc295b2e2b52..4aa949799446 100644
--- a/drivers/gpu/drm/qxl/qxl_prime.c
+++ b/drivers/gpu/drm/qxl/qxl_prime.c
@@ -59,7 +59,7 @@ int qxl_gem_prime_vmap(struct drm_gem_object *obj, struct
dma_buf_map *map)
struct qxl_bo *bo = gem_to_qxl_bo(obj);
int ret;
- ret = qxl_bo_kmap_locked(bo, map);
+ ret = qxl_bo_kmap(bo, map);
if (ret < 0)
return ret;
@@ -71,7 +71,7 @@ void qxl_gem_prime_vunmap(struct drm_gem_object *obj,
{
struct qxl_bo *bo = gem_to_qxl_bo(obj);
- qxl_bo_kunmap_locked(bo);
+ qxl_bo_kunmap(bo);
}
int qxl_gem_prime_mmap(struct drm_gem_object *obj,
--
2.29.2
[PATCH 03/10] drm/qxl: use ttm bo priorities
Allow to set priorities for buffer objects. Use priority 1 for surface
and cursor command releases. Use priority 0 for drawing command
releases. That way the short-living drawing commands are first in line
when it comes to eviction, making it *much* less likely that
ttm_bo_mem_force_space() picks something which can't be evicted and
throws an error after waiting a while without success.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_object.h | 1 +
drivers/gpu/drm/qxl/qxl_cmd.c | 2 +-
drivers/gpu/drm/qxl/qxl_display.c | 4 ++--
drivers/gpu/drm/qxl/qxl_gem.c | 2 +-
drivers/gpu/drm/qxl/qxl_object.c | 5 +++--
drivers/gpu/drm/qxl/qxl_release.c | 19 +--
6 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_object.h b/drivers/gpu/drm/qxl/qxl_object.h
index e60a8f88e226..dc1659e717f1 100644
--- a/drivers/gpu/drm/qxl/qxl_object.h
+++ b/drivers/gpu/drm/qxl/qxl_object.h
@@ -61,6 +61,7 @@ static inline u64 qxl_bo_mmap_offset(struct qxl_bo *bo)
extern int qxl_bo_create(struct qxl_device *qdev,
unsigned long size,
bool kernel, bool pinned, u32 domain,
+u32 priority,
struct qxl_surface *surf,
struct qxl_bo **bo_ptr);
extern int qxl_bo_kmap(struct qxl_bo *bo, struct dma_buf_map *map);
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 7e22a81bfb36..7b00c955cd82 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -269,7 +269,7 @@ int qxl_alloc_bo_reserved(struct qxl_device *qdev,
int ret;
ret = qxl_bo_create(qdev, size, false /* not kernel - device */,
- false, QXL_GEM_DOMAIN_VRAM, NULL, &bo);
+ false, QXL_GEM_DOMAIN_VRAM, 0, NULL, &bo);
if (ret) {
DRM_ERROR("failed to allocate VRAM BO\n");
return ret;
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index ec50d2cfd4e1..a1b5cc5918bc 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -799,8 +799,8 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
qdev->dumb_shadow_bo = NULL;
}
qxl_bo_create(qdev, surf.height * surf.stride,
- true, true, QXL_GEM_DOMAIN_SURFACE, &surf,
- &qdev->dumb_shadow_bo);
+ true, true, QXL_GEM_DOMAIN_SURFACE, 0,
+ &surf, &qdev->dumb_shadow_bo);
}
if (user_bo->shadow != qdev->dumb_shadow_bo) {
if (user_bo->shadow) {
diff --git a/drivers/gpu/drm/qxl/qxl_gem.c b/drivers/gpu/drm/qxl/qxl_gem.c
index 48e096285b4c..a08da0bd9098 100644
--- a/drivers/gpu/drm/qxl/qxl_gem.c
+++ b/drivers/gpu/drm/qxl/qxl_gem.c
@@ -55,7 +55,7 @@ int qxl_gem_object_create(struct qxl_device *qdev, int size,
/* At least align on page size */
if (alignment < PAGE_SIZE)
alignment = PAGE_SIZE;
- r = qxl_bo_create(qdev, size, kernel, false, initial_domain, surf,
&qbo);
+ r = qxl_bo_create(qdev, size, kernel, false, initial_domain, 0, surf,
&qbo);
if (r) {
if (r != -ERESTARTSYS)
DRM_ERROR(
diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
index 705b51535492..7eada4ad217b 100644
--- a/drivers/gpu/drm/qxl/qxl_object.c
+++ b/drivers/gpu/drm/qxl/qxl_object.c
@@ -103,8 +103,8 @@ static const struct drm_gem_object_funcs qxl_object_funcs =
{
.print_info = drm_gem_ttm_print_info,
};
-int qxl_bo_create(struct qxl_device *qdev,
- unsigned long size, bool kernel, bool pinned, u32 domain,
+int qxl_bo_create(struct qxl_device *qdev, unsigned long size,
+ bool kernel, bool pinned, u32 domain, u32 priority,
struct qxl_surface *surf,
struct qxl_bo **bo_ptr)
{
@@ -137,6 +137,7 @@ int qxl_bo_create(struct qxl_device *qdev,
qxl_ttm_placement_from_domain(bo, domain);
+ bo->tbo.priority = priority;
r = ttm_bo_init_reserved(&qdev->mman.bdev, &bo->tbo, size, type,
&bo->placement, 0, &ctx, NULL, NULL,
&qxl_ttm_bo_destroy);
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 63818b56218c..a831184e014a 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -161,11 +161,12 @@ qxl_release_free(struct qxl_device *qdev,
}
static int qxl_release_bo_alloc(struct qxl_device *qdev,
- struct
[PATCH 02/10] drm/qxl: more fence wait rework
Move qxl_io_notify_oom() call into wait condition.
That way the driver will call it again if one call
wasn't enough.
Also allows to remove the extra dma_fence_is_signaled()
check and the goto.
Fixes: 5a838e5d5825 ("drm/qxl: simplify qxl_fence_wait")
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_release.c | 9 +++--
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 6ed673d75f9f..63818b56218c 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -62,16 +62,13 @@ static long qxl_fence_wait(struct dma_fence *fence, bool
intr,
qdev = container_of(fence->lock, struct qxl_device, release_lock);
- if (dma_fence_is_signaled(fence))
- goto signaled;
-
- qxl_io_notify_oom(qdev);
if (!wait_event_timeout(qdev->release_event,
- dma_fence_is_signaled(fence),
+ (dma_fence_is_signaled(fence) || (
+ qxl_io_notify_oom(qdev),
+ 0)),
timeout))
return 0;
-signaled:
cur = jiffies;
if (time_after(cur, end))
return 0;
--
2.29.2
Re: [PATCH] drm/qxl: properly handle device init failures
On Mon, Feb 08, 2021 at 12:07:01PM -0500, Tong Zhang wrote: > Does this patch fix an issue raised previously? Or should they be used > together? > https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2466541.html > > IMHO using this patch alone won’t fix the issue This patch on top of drm-misc-next fixes the initialization error issue reported by you in my testing. take care, Gerd
[PATCH] drm/qxl: properly handle device init failures
Specifically do not try release resources which where
not allocated in the first place.
Cc: Tong Zhang
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 3 +++
drivers/gpu/drm/qxl/qxl_kms.c | 4
2 files changed, 7 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index c326412136c5..ec50d2cfd4e1 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1183,6 +1183,9 @@ int qxl_destroy_monitors_object(struct qxl_device *qdev)
{
int ret;
+ if (!qdev->monitors_config_bo)
+ return 0;
+
qdev->monitors_config = NULL;
qdev->ram_header->monitors_config = 0;
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 66d74aaaee06..4dc5ad13f12c 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -288,6 +288,10 @@ void qxl_device_fini(struct qxl_device *qdev)
{
int cur_idx;
+ /* check if qxl_device_init() was successful (gc_work is initialized
last) */
+ if (!qdev->gc_work.func)
+ return;
+
for (cur_idx = 0; cur_idx < 3; cur_idx++) {
if (!qdev->current_release_bo[cur_idx])
continue;
--
2.29.2
Re: [PATCH v2] drm/qxl: do not run release if qxl failed to init
On Thu, Feb 04, 2021 at 11:30:50AM -0500, Tong Zhang wrote: > if qxl_device_init() fail, drm device will not be registered, > in this case, do not run qxl_drm_release() How do you trigger this? take care, Gerd
Re: [PATCH v6 01/10] [hack] silence ttm fini WARNING
On Thu, Feb 04, 2021 at 03:58:33PM +0100, Christian König wrote:
> ?
>
> What's the background here?
>
> Christian.
>
> Am 04.02.21 um 15:57 schrieb Gerd Hoffmann:
> > kobject: '(null)' ((ptrval)): is not initialized, yet kobject_put()
> > is being called.
> > WARNING: CPU: 0 PID: 209 at lib/kobject.c:750 kobject_put+0x3a/0x60
> > [ ... ]
> > Call Trace:
> > ttm_device_fini+0x133/0x1b0 [ttm]
> > qxl_ttm_fini+0x2f/0x40 [qxl]
Happens on driver removal. Seen with both qxl and bochs (the later
using vram helpers).
Testcase: "drmtest --unbind" (https://git.kraxel.org/cgit/drminfo/).
static int try_unbind(int card)
{
char path[256];
char *device, *name;
int fd;
snprintf(path, sizeof(path), "/sys/class/drm/card%d/device", card);
device = realpath(path, NULL);
if (device == NULL) {
fprintf(stderr, "%s: can't resolve sysfs device path\n", __func__);
return -1;
}
snprintf(path, sizeof(path), "%s/driver/unbind", device);
fd = open(path, O_WRONLY);
if (fd < 0) {
fprintf(stderr, "open %s: %s\n", path, strerror(errno));
return -1;
}
name = strrchr(device, '/') + 1;
write(fd, name, strlen(name));
close(fd);
return 0;
}
take care,
Gerd
[PATCH v6 08/10] drm/qxl: properly free qxl releases
Reorganize qxl_device_fini() a bit.
Add missing unpin() calls.
Count releases. Add wait queue for releases. That way
qxl_device_fini() can easily wait until everything is
ready for proper shutdown.
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_drv.h | 2 ++
drivers/gpu/drm/qxl/qxl_cmd.c | 1 +
drivers/gpu/drm/qxl/qxl_irq.c | 1 +
drivers/gpu/drm/qxl/qxl_kms.c | 28
drivers/gpu/drm/qxl/qxl_release.c | 2 ++
5 files changed, 30 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 01354b43c413..6dd57cfb2e7c 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -214,6 +214,8 @@ struct qxl_device {
spinlock_t release_lock;
struct idr release_idr;
uint32_trelease_seqno;
+ atomic_trelease_count;
+ wait_queue_head_t release_event;
spinlock_t release_idr_lock;
struct mutexasync_io_mutex;
unsigned int last_sent_io_cmd;
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 54e3c3a97440..7e22a81bfb36 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -254,6 +254,7 @@ int qxl_garbage_collect(struct qxl_device *qdev)
}
}
+ wake_up_all(&qdev->release_event);
DRM_DEBUG_DRIVER("%d\n", i);
return i;
diff --git a/drivers/gpu/drm/qxl/qxl_irq.c b/drivers/gpu/drm/qxl/qxl_irq.c
index ddf6588a2a38..d312322cacd1 100644
--- a/drivers/gpu/drm/qxl/qxl_irq.c
+++ b/drivers/gpu/drm/qxl/qxl_irq.c
@@ -87,6 +87,7 @@ int qxl_irq_init(struct qxl_device *qdev)
init_waitqueue_head(&qdev->display_event);
init_waitqueue_head(&qdev->cursor_event);
init_waitqueue_head(&qdev->io_cmd_event);
+ init_waitqueue_head(&qdev->release_event);
INIT_WORK(&qdev->client_monitors_config_work,
qxl_client_monitors_config_work_func);
atomic_set(&qdev->irq_received, 0);
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 4a60a52ab62e..66d74aaaee06 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -286,11 +286,31 @@ int qxl_device_init(struct qxl_device *qdev,
void qxl_device_fini(struct qxl_device *qdev)
{
- qxl_bo_unref(&qdev->current_release_bo[0]);
- qxl_bo_unref(&qdev->current_release_bo[1]);
- qxl_gem_fini(qdev);
- qxl_bo_fini(qdev);
+ int cur_idx;
+
+ 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(&qdev->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);
+ wait_event_timeout(qdev->release_event,
+ atomic_read(&qdev->release_count) == 0,
+ HZ);
flush_work(&qdev->gc_work);
+ qxl_surf_evict(qdev);
+ qxl_vram_evict(qdev);
+
+ qxl_gem_fini(qdev);
+ qxl_bo_fini(qdev);
qxl_ring_free(qdev->command_ring);
qxl_ring_free(qdev->cursor_ring);
qxl_ring_free(qdev->release_ring);
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(&qdev->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(&qdev->release_count);
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
--
2.29.2
[PATCH v6 10/10] drm/qxl: allocate dumb buffers in ram
dumb buffers are shadowed anyway, so there is no need to store them in device memory. Use QXL_GEM_DOMAIN_CPU (TTM_PL_SYSTEM) instead. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/qxl/qxl_dumb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/qxl/qxl_dumb.c b/drivers/gpu/drm/qxl/qxl_dumb.c index c04cd5a2553c..48a58ba1db96 100644 --- a/drivers/gpu/drm/qxl/qxl_dumb.c +++ b/drivers/gpu/drm/qxl/qxl_dumb.c @@ -59,7 +59,7 @@ int qxl_mode_dumb_create(struct drm_file *file_priv, surf.stride = pitch; surf.format = format; r = qxl_gem_object_create_with_handle(qdev, file_priv, - QXL_GEM_DOMAIN_SURFACE, + QXL_GEM_DOMAIN_CPU, args->size, &surf, &qobj, &handle); if (r) -- 2.29.2
[PATCH v6 09/10] drm/qxl: simplify qxl_fence_wait
Now that we have the new release_event wait queue we can just
use that in qxl_fence_wait() and simplify the code alot.
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_release.c | 44 +++
1 file changed, 4 insertions(+), 40 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 43a5436853b7..6ed673d75f9f 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -58,54 +58,18 @@ static long qxl_fence_wait(struct dma_fence *fence, bool
intr,
signed long timeout)
{
struct qxl_device *qdev;
- struct qxl_release *release;
- int count = 0, sc = 0;
- bool have_drawable_releases;
unsigned long cur, end = jiffies + timeout;
qdev = container_of(fence->lock, struct qxl_device, release_lock);
- release = container_of(fence, struct qxl_release, base);
- have_drawable_releases = release->type == QXL_RELEASE_DRAWABLE;
-
-retry:
- sc++;
if (dma_fence_is_signaled(fence))
goto signaled;
qxl_io_notify_oom(qdev);
-
- for (count = 0; count < 11; count++) {
- if (!qxl_queue_garbage_collect(qdev, true))
- break;
-
- if (dma_fence_is_signaled(fence))
- goto signaled;
- }
-
- if (dma_fence_is_signaled(fence))
- goto signaled;
-
- if (have_drawable_releases || sc < 4) {
- if (sc > 2)
- /* back off */
- usleep_range(500, 1000);
-
- if (time_after(jiffies, end))
- return 0;
-
- if (have_drawable_releases && sc > 300) {
- DMA_FENCE_WARN(fence, "failed to wait on release %llu "
- "after spincount %d\n",
- fence->context & ~0xf000, sc);
- goto signaled;
- }
- goto retry;
- }
- /*
-* yeah, original sync_obj_wait gave up after 3 spins when
-* have_drawable_releases is not set.
-*/
+ if (!wait_event_timeout(qdev->release_event,
+ dma_fence_is_signaled(fence),
+ timeout))
+ return 0;
signaled:
cur = jiffies;
--
2.29.2
[PATCH v6 07/10] drm/qxl: handle shadow in primary destroy
qxl_primary_atomic_disable must check whenever the framebuffer bo has a
shadow surface and in case it has check the shadow primary status.
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
drivers/gpu/drm/qxl/qxl_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index d25fd3acc891..c326412136c5 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -562,6 +562,8 @@ static void qxl_primary_atomic_disable(struct drm_plane
*plane,
if (old_state->fb) {
struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
+ if (bo->shadow)
+ bo = bo->shadow;
if (bo->is_primary) {
qxl_io_destroy_primary(qdev);
bo->is_primary = false;
--
2.29.2
[PATCH v6 03/10] drm/qxl: use drmm_mode_config_init
Signed-off-by: Gerd Hoffmann
Reviewed-by: Daniel Vetter
Acked-by: Thomas Zimmermann
---
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 012bce0cdb65..38d6b596094d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1195,7 +1195,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
int i;
int ret;
- drm_mode_config_init(&qdev->ddev);
+ ret = drmm_mode_config_init(&qdev->ddev);
+ if (ret)
+ return ret;
ret = qxl_create_monitors_object(qdev);
if (ret)
@@ -1228,5 +1230,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(&qdev->ddev);
}
--
2.29.2
[PATCH v6 06/10] drm/qxl: properly pin/unpin shadow
Suggested-by: Thomas Zimmermann
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 60331e31861a..d25fd3acc891 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -802,12 +802,14 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane,
}
if (user_bo->shadow != qdev->dumb_shadow_bo) {
if (user_bo->shadow) {
+ qxl_bo_unpin(user_bo->shadow);
drm_gem_object_put
(&user_bo->shadow->tbo.base);
user_bo->shadow = NULL;
}
drm_gem_object_get(&qdev->dumb_shadow_bo->tbo.base);
user_bo->shadow = qdev->dumb_shadow_bo;
+ qxl_bo_pin(user_bo->shadow);
}
}
@@ -833,6 +835,7 @@ static void qxl_plane_cleanup_fb(struct drm_plane *plane,
qxl_bo_unpin(user_bo);
if (old_state->fb != plane->state->fb && user_bo->shadow) {
+ qxl_bo_unpin(user_bo->shadow);
drm_gem_object_put(&user_bo->shadow->tbo.base);
user_bo->shadow = NULL;
}
@@ -1230,6 +1233,7 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
if (qdev->dumb_shadow_bo) {
+ qxl_bo_unpin(qdev->dumb_shadow_bo);
drm_gem_object_put(&qdev->dumb_shadow_bo->tbo.base);
qdev->dumb_shadow_bo = NULL;
}
--
2.29.2
[PATCH v6 02/10] Revert "drm/qxl: do not run release if qxl failed to init"
This reverts commit b91907a6241193465ca92e357adf16822242296d. Patch is broken, it effectively makes qxl_drm_release() a nop because on normal driver shutdown qxl_drm_release() is called *after* drm_dev_unregister(). Cc: Tong Zhang Signed-off-by: Gerd Hoffmann --- 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 34c8b25b5780..fb5f6a5e81d7 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) * reodering qxl_modeset_fini() + qxl_device_fini() calls is * non-trivial though. */ - if (!dev->registered) - return; qxl_modeset_fini(qdev); qxl_device_fini(qdev); } -- 2.29.2
[PATCH v6 01/10] [hack] silence ttm fini WARNING
kobject: '(null)' ((ptrval)): is not initialized, yet kobject_put() is being called. WARNING: CPU: 0 PID: 209 at lib/kobject.c:750 kobject_put+0x3a/0x60 [ ... ] Call Trace: ttm_device_fini+0x133/0x1b0 [ttm] qxl_ttm_fini+0x2f/0x40 [qxl] --- drivers/gpu/drm/ttm/ttm_device.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c index ac0903c9e60a..b638cbb0bd45 100644 --- a/drivers/gpu/drm/ttm/ttm_device.c +++ b/drivers/gpu/drm/ttm/ttm_device.c @@ -50,7 +50,7 @@ static void ttm_global_release(void) goto out; kobject_del(&glob->kobj); - kobject_put(&glob->kobj); +// kobject_put(&glob->kobj); ttm_mem_global_release(&ttm_mem_glob); __free_page(glob->dummy_read_page); memset(glob, 0, sizeof(*glob)); -- 2.29.2
[PATCH v6 05/10] drm/qxl: release shadow on shutdown
In case we have a shadow surface on shutdown release
it so it doesn't leak.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 38d6b596094d..60331e31861a 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1229,5 +1229,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
+ if (qdev->dumb_shadow_bo) {
+ drm_gem_object_put(&qdev->dumb_shadow_bo->tbo.base);
+ qdev->dumb_shadow_bo = NULL;
+ }
qxl_destroy_monitors_object(qdev);
}
--
2.29.2
[PATCH v6 04/10] drm/qxl: unpin release objects
Balances the qxl_create_bo(..., pinned=true, ...);
call in qxl_release_bo_alloc().
Signed-off-by: Gerd Hoffmann
Acked-by: Thomas Zimmermann
---
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 c52412724c26..28013fd1f8ea 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -347,6 +347,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
+ qxl_bo_unpin(qdev->current_release_bo[cur_idx]);
qxl_bo_unref(&qdev->current_release_bo[cur_idx]);
qdev->current_release_bo_offset[cur_idx] = 0;
qdev->current_release_bo[cur_idx] = NULL;
--
2.29.2
Re: [PATCH V2] virtio_input: Prevent EV_MSC/MSC_TIMESTAMP loop storm for MT.
Hi, > + /* > + * Since 29cc309d8bf1 (HID: hid-multitouch: forward MSC_TIMESTAMP), > + * EV_MSC/MSC_TIMESTAMP is added to each before EV_SYN event. > + * EV_MSC is configured as INPUT_PASS_TO_ALL. > + * In case of touch device: > + * BE pass EV_MSC/MSC_TIMESTAMP to FE on receiving event from evdev. > + * FE pass EV_MSC/MSC_TIMESTAMP back to BE. > + * BE writes EV_MSC/MSC_TIMESTAMP to evdev due to INPUT_PASS_TO_ALL. > + * BE receives extra EV_MSC/MSC_TIMESTAMP and pass to FE. > + * >>> Each new frame becomes larger and larger. > + * Disable EV_MSC/MSC_TIMESTAMP forwarding for MT. > + */ > + if (vi->idev->mt && type == EV_MSC && code == MSC_TIMESTAMP) > + return 0; > + Reviewed-by: Gerd Hoffmann take care, Gerd
Re: [PATCH RESEND v3] virtio-input: add multi-touch support
On Fri, Jan 15, 2021 at 02:26:23AM +0200, Vasyl Vavrychuk wrote: > From: Mathias Crombez > > Without multi-touch slots allocated, ABS_MT_SLOT events will be lost by > input_handle_abs_event. > > Implementation is based on uinput_create_device. > > Signed-off-by: Mathias Crombez > Co-developed-by: Vasyl Vavrychuk > Signed-off-by: Vasyl Vavrychuk > --- > v2: fix patch corrupted by corporate email server > v3: use number of slots from the host > > Resend since to feedback. > > drivers/virtio/virtio_input.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) Have no test hardware, the logic looks sane though. Reviewed-by: Gerd Hoffmann take care, Gerd
[PATCH v5 3/6] drm/qxl: release shadow on shutdown
In case we have a shadow surface on shutdown release
it so it doesn't leak.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 38d6b596094d..60331e31861a 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1229,5 +1229,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
+ if (qdev->dumb_shadow_bo) {
+ drm_gem_object_put(&qdev->dumb_shadow_bo->tbo.base);
+ qdev->dumb_shadow_bo = NULL;
+ }
qxl_destroy_monitors_object(qdev);
}
--
2.29.2
[PATCH v5 5/6] drm/qxl: properly free qxl releases
Reorganize qxl_device_fini() a bit.
Add missing unpin() calls.
Count releases. Add wait queue for releases. That way
qxl_device_fini() can easily wait until everything is
ready for proper shutdown.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_drv.h | 2 ++
drivers/gpu/drm/qxl/qxl_cmd.c | 1 +
drivers/gpu/drm/qxl/qxl_irq.c | 1 +
drivers/gpu/drm/qxl/qxl_kms.c | 22 --
drivers/gpu/drm/qxl/qxl_release.c | 2 ++
5 files changed, 26 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 01354b43c413..6dd57cfb2e7c 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -214,6 +214,8 @@ struct qxl_device {
spinlock_t release_lock;
struct idr release_idr;
uint32_trelease_seqno;
+ atomic_trelease_count;
+ wait_queue_head_t release_event;
spinlock_t release_idr_lock;
struct mutexasync_io_mutex;
unsigned int last_sent_io_cmd;
diff --git a/drivers/gpu/drm/qxl/qxl_cmd.c b/drivers/gpu/drm/qxl/qxl_cmd.c
index 54e3c3a97440..7e22a81bfb36 100644
--- a/drivers/gpu/drm/qxl/qxl_cmd.c
+++ b/drivers/gpu/drm/qxl/qxl_cmd.c
@@ -254,6 +254,7 @@ int qxl_garbage_collect(struct qxl_device *qdev)
}
}
+ wake_up_all(&qdev->release_event);
DRM_DEBUG_DRIVER("%d\n", i);
return i;
diff --git a/drivers/gpu/drm/qxl/qxl_irq.c b/drivers/gpu/drm/qxl/qxl_irq.c
index ddf6588a2a38..d312322cacd1 100644
--- a/drivers/gpu/drm/qxl/qxl_irq.c
+++ b/drivers/gpu/drm/qxl/qxl_irq.c
@@ -87,6 +87,7 @@ int qxl_irq_init(struct qxl_device *qdev)
init_waitqueue_head(&qdev->display_event);
init_waitqueue_head(&qdev->cursor_event);
init_waitqueue_head(&qdev->io_cmd_event);
+ init_waitqueue_head(&qdev->release_event);
INIT_WORK(&qdev->client_monitors_config_work,
qxl_client_monitors_config_work_func);
atomic_set(&qdev->irq_received, 0);
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 4a60a52ab62e..616aea948863 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -286,8 +286,26 @@ int qxl_device_init(struct qxl_device *qdev,
void qxl_device_fini(struct qxl_device *qdev)
{
- qxl_bo_unref(&qdev->current_release_bo[0]);
- qxl_bo_unref(&qdev->current_release_bo[1]);
+ int cur_idx;
+
+ 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(&qdev->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);
+ wait_event_timeout(qdev->release_event,
+ atomic_read(&qdev->release_count) == 0,
+ HZ);
+
qxl_gem_fini(qdev);
qxl_bo_fini(qdev);
flush_work(&qdev->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(&qdev->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(&qdev->release_count);
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
--
2.29.2
[PATCH v5 4/6] drm/qxl: handle shadow in primary destroy
qxl_primary_atomic_disable must check whenever the framebuffer bo has a
shadow surface and in case it has check the shadow primary status.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 60331e31861a..f5ee8cd72b5b 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -562,6 +562,8 @@ static void qxl_primary_atomic_disable(struct drm_plane
*plane,
if (old_state->fb) {
struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
+ if (bo->shadow)
+ bo = bo->shadow;
if (bo->is_primary) {
qxl_io_destroy_primary(qdev);
bo->is_primary = false;
--
2.29.2
[PATCH v5 1/6] drm/qxl: use drmm_mode_config_init
Signed-off-by: Gerd Hoffmann
Reviewed-by: Daniel Vetter
Acked-by: Thomas Zimmermann
---
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 012bce0cdb65..38d6b596094d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1195,7 +1195,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
int i;
int ret;
- drm_mode_config_init(&qdev->ddev);
+ ret = drmm_mode_config_init(&qdev->ddev);
+ if (ret)
+ return ret;
ret = qxl_create_monitors_object(qdev);
if (ret)
@@ -1228,5 +1230,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(&qdev->ddev);
}
--
2.29.2
[PATCH v5 6/6] drm/qxl: simplify qxl_fence_wait
Now that we have the new release_event wait queue we can just
use that in qxl_fence_wait() and simplify the code alot.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_release.c | 42 +++
1 file changed, 4 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/qxl/qxl_release.c
b/drivers/gpu/drm/qxl/qxl_release.c
index 43a5436853b7..b6f4b8dcf228 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -59,53 +59,19 @@ static long qxl_fence_wait(struct dma_fence *fence, bool
intr,
{
struct qxl_device *qdev;
struct qxl_release *release;
- int count = 0, sc = 0;
- bool have_drawable_releases;
unsigned long cur, end = jiffies + timeout;
qdev = container_of(fence->lock, struct qxl_device, release_lock);
release = container_of(fence, struct qxl_release, base);
- have_drawable_releases = release->type == QXL_RELEASE_DRAWABLE;
-
-retry:
- sc++;
if (dma_fence_is_signaled(fence))
goto signaled;
qxl_io_notify_oom(qdev);
-
- for (count = 0; count < 11; count++) {
- if (!qxl_queue_garbage_collect(qdev, true))
- break;
-
- if (dma_fence_is_signaled(fence))
- goto signaled;
- }
-
- if (dma_fence_is_signaled(fence))
- goto signaled;
-
- if (have_drawable_releases || sc < 4) {
- if (sc > 2)
- /* back off */
- usleep_range(500, 1000);
-
- if (time_after(jiffies, end))
- return 0;
-
- if (have_drawable_releases && sc > 300) {
- DMA_FENCE_WARN(fence, "failed to wait on release %llu "
- "after spincount %d\n",
- fence->context & ~0xf000, sc);
- goto signaled;
- }
- goto retry;
- }
- /*
-* yeah, original sync_obj_wait gave up after 3 spins when
-* have_drawable_releases is not set.
-*/
+ if (!wait_event_timeout(qdev->release_event,
+ dma_fence_is_signaled(fence),
+ timeout))
+ return 0;
signaled:
cur = jiffies;
--
2.29.2
[PATCH v5 2/6] drm/qxl: unpin release objects
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 c52412724c26..28013fd1f8ea 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -347,6 +347,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
+ qxl_bo_unpin(qdev->current_release_bo[cur_idx]);
qxl_bo_unref(&qdev->current_release_bo[cur_idx]);
qdev->current_release_bo_offset[cur_idx] = 0;
qdev->current_release_bo[cur_idx] = NULL;
--
2.29.2
Re: [PATCH v4 5/5] drm/qxl: properly free qxl releases
> > + /* > > +* 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(&qdev->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. There isn't one ready for use, and adding a new one (to wait for the garbage collection having released something) just for a clean shutdown looked a bit like overkill. But after digging a bit more and looking at the qxl_fence_wait() mess I think adding a wait queue looks like a good idea ... v5 coming soon ... take care, Gerd
[PATCH v4 3/5] drm/qxl: release shadow on shutdown
In case we have a shadow surface on shutdown release
it so it doesn't leak.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 38d6b596094d..60331e31861a 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1229,5 +1229,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
+ if (qdev->dumb_shadow_bo) {
+ drm_gem_object_put(&qdev->dumb_shadow_bo->tbo.base);
+ qdev->dumb_shadow_bo = NULL;
+ }
qxl_destroy_monitors_object(qdev);
}
--
2.29.2
[PATCH v4 5/5] drm/qxl: properly free qxl releases
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(&qdev->current_release_bo[0]);
- qxl_bo_unref(&qdev->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(&qdev->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(&qdev->release_count) > 0; try++)
+ msleep(20);
+
qxl_gem_fini(qdev);
qxl_bo_fini(qdev);
flush_work(&qdev->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(&qdev->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(&qdev->release_count);
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
--
2.29.2
[PATCH v4 1/5] drm/qxl: use drmm_mode_config_init
Signed-off-by: Gerd Hoffmann
Reviewed-by: Daniel Vetter
Acked-by: Thomas Zimmermann
---
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 012bce0cdb65..38d6b596094d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1195,7 +1195,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
int i;
int ret;
- drm_mode_config_init(&qdev->ddev);
+ ret = drmm_mode_config_init(&qdev->ddev);
+ if (ret)
+ return ret;
ret = qxl_create_monitors_object(qdev);
if (ret)
@@ -1228,5 +1230,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(&qdev->ddev);
}
--
2.29.2
[PATCH v4 4/5] drm/qxl: handle shadow in primary destroy
qxl_primary_atomic_disable must check whenever the framebuffer bo has a
shadow surface and in case it has check the shadow primary status.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 60331e31861a..f5ee8cd72b5b 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -562,6 +562,8 @@ static void qxl_primary_atomic_disable(struct drm_plane
*plane,
if (old_state->fb) {
struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
+ if (bo->shadow)
+ bo = bo->shadow;
if (bo->is_primary) {
qxl_io_destroy_primary(qdev);
bo->is_primary = false;
--
2.29.2
[PATCH v4 2/5] drm/qxl: unpin release objects
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 c52412724c26..28013fd1f8ea 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -347,6 +347,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
+ qxl_bo_unpin(qdev->current_release_bo[cur_idx]);
qxl_bo_unref(&qdev->current_release_bo[cur_idx]);
qdev->current_release_bo_offset[cur_idx] = 0;
qdev->current_release_bo[cur_idx] = NULL;
--
2.29.2
Re: [PATCH v3 2/4] drm/qxl: unpin release objects
> > 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.
Figured the unpin is at the completely wrong place while trying to
reproduce the lockdep splat ...
take care,
Gerd
>From 43befab4a935114e8620af62781666fa81288255 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann
Date: Mon, 25 Jan 2021 13:10:50 +0100
Subject: [PATCH] drm/qxl: unpin release objects
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 c52412724c26..28013fd1f8ea 100644
--- a/drivers/gpu/drm/qxl/qxl_release.c
+++ b/drivers/gpu/drm/qxl/qxl_release.c
@@ -347,6 +347,7 @@ int qxl_alloc_release_reserved(struct qxl_device *qdev,
unsigned long size,
mutex_lock(&qdev->release_mutex);
if (qdev->current_release_bo_offset[cur_idx] + 1 >=
releases_per_bo[cur_idx]) {
+ qxl_bo_unpin(qdev->current_release_bo[cur_idx]);
qxl_bo_unref(&qdev->current_release_bo[cur_idx]);
qdev->current_release_bo_offset[cur_idx] = 0;
qdev->current_release_bo[cur_idx] = NULL;
--
2.29.2
Re: [PATCH v3 2/4] drm/qxl: unpin release objects
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(&bo->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. 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
Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver
On Wed, Jan 20, 2021 at 10:52:11AM -0800, Yiwei Zhang wrote:
> On Wed, Jan 20, 2021 at 5:33 AM Gerd Hoffmann wrote:
> >
> > Hi,
> >
> > > > > > > + select TRACE_GPU_MEM
> >
> > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM
> >
> > That doesn't make sense btw.
>
> Do you recommend we just select it or leave it an option?
The patch selects it (which makes sense given the small size).
The #ifdef is pointless then ...
> > > > > > > +#ifdef CONFIG_TRACE_GPU_MEM
> > > > > > > +static inline void virtio_gpu_trace_total_mem(struct
> > > > > > > virtio_gpu_device *vgdev,
> > > > > > > + s64 delta)
> > > > > > > +{
> > > > > > > + u64 total_mem = atomic64_add_return(delta,
> > > > > > > &vgdev->total_mem);
> > > > > > > +
> > > > > > > + trace_gpu_mem_total(0, 0, total_mem);
> >
> > Hmm, so no per process tracking (pid arg hard-coded to zero)?
> > Any plans for that?
> > The cgroups patches mentioned by Daniel should address that btw.
>
> Android GPU vendors do report the totals for each process as well. For
> Cuttlefish virtual platform, we haven't yet required that, and want to
> get the global total in place first.
That means no plans yet?
> > > > > Android relies on this tracepoint + eBPF to make the GPU memory totals
> > > > > available at runtime on production devices, which has been enforced
> > > > > already. Not only game developers can have a reliable kernel total GPU
> > > > > memory to look at, but also Android leverages this to take GPU memory
> > > > > usage out from the system lost ram.
> >
> > Sounds like you define "gpu memory" as "system memory used to store gpu
> > data". Is that correct? What about device memory?
>
> The total definition does include all device memory being used as well
> for numa devices.(If my understanding of your question is correct.)
device memory == gpu-owned memory, typically exposed to as pci memory bar.
qemu stdvga for example stores gem objects in device memory (unless it
runs out of vram, then ttm allocates from / moves into main memory).
> > > > > I'm not sure whether the other DRM drivers would like to integrate
> > > > > this tracepoint(maybe upstream drivers will move away from debugfs
> > > > > later as well?), but at least we hope virtio-gpu can take this.
> >
> > Well, it is basically the same for all drivers using the gem shmem
> > helpers. So I see little reason why we should do that at virtio-gpu
> > level.
>
> This can be a starting point. Another reason would be I'm fearing that
> this tracepoint approach might be more difficult to get upstreamed at
> drm layer level, since later we may want to get to per-process total
> tracking, which would be making more sense at device driver level.
Tracking in __drm_gem_shmem_create + drm_gem_shmem_free_object should
give you pretty much the same results, with the major difference being
that it works for all shmem-based drivers.
Of course just moving the trace points doesn't solve the other issues
discussed.
> > > Android GPU vendors have integrated this tracepoint to track gpu
> > > memory usage total(mapped into the gpu address space), which consists
> > > of below:
> > > (1) directly allocated via physical page allocator
> > > (2) imported external memory backed by dma-bufs
> > > (3) allocated exportable memory backed by dma-bufs
> >
> > Hmm, the tracepoint doesn't track which of the three groups the memory
> > belongs to. Which I think is important, specifically group (2) because
> > that might already be accounted for by the exporting driver ...
>
> The tracepoint only cares about a total number, but I'm not against
> the idea to extend the tracepoint with categorization. However, I
> believe the dma-bufs core can track which dma-buf gets attached/mapped
> to some devices. So that those overlap between dma-buf heaps and the
> gpu memory total we are tracking here can be canceled out.
Yep, maybe. Which is *exactly* why Daniel keeps asking for the big
picture and how this integrates/interacts with the dma-buf accounting
which seems to be in the works too.
Note that dma-bufs are not only used for cross-device sharing. They are
also used to pass handles from one application to another (application
to wayland compositor or x server for example). Which doesn't matter
much for the to
Re: [PATCH v2] drm/virtio: Track total GPU memory for virtio driver
Hi,
> > > > > + select TRACE_GPU_MEM
> > > > > +#ifdef CONFIG_TRACE_GPU_MEM
That doesn't make sense btw.
> > > > > +#ifdef CONFIG_TRACE_GPU_MEM
> > > > > +static inline void virtio_gpu_trace_total_mem(struct
> > > > > virtio_gpu_device *vgdev,
> > > > > + s64 delta)
> > > > > +{
> > > > > + u64 total_mem = atomic64_add_return(delta, &vgdev->total_mem);
> > > > > +
> > > > > + trace_gpu_mem_total(0, 0, total_mem);
Hmm, so no per process tracking (pid arg hard-coded to zero)?
Any plans for that?
The cgroups patches mentioned by Daniel should address that btw.
The gpu_id is hardcoded to zero too. Shouldn't that be something like
the minor number of the drm device? Or maybe something else in case you
need drm and non-drm gpu devices work side-by-side?
> > > Thanks for your reply! Android Cuttlefish virtual platform is using
> > > the virtio-gpu driver, and we currently are carrying this small patch
> > > at the downstream side. This is essential for us because:
> > > (1) Android has deprecated debugfs on production devices already
IIRC there have been discussions about a statfs, so you can export stats
with a sane interface without also enabling all the power provided by
debugfs, exactly because of the concerns to do that on production
systems.
Not sure what the state is, seems to not be upstream yet. That would be
(beside cgroups) another thing to look at.
> > > Android relies on this tracepoint + eBPF to make the GPU memory totals
> > > available at runtime on production devices, which has been enforced
> > > already. Not only game developers can have a reliable kernel total GPU
> > > memory to look at, but also Android leverages this to take GPU memory
> > > usage out from the system lost ram.
Sounds like you define "gpu memory" as "system memory used to store gpu
data". Is that correct? What about device memory?
> > > I'm not sure whether the other DRM drivers would like to integrate
> > > this tracepoint(maybe upstream drivers will move away from debugfs
> > > later as well?), but at least we hope virtio-gpu can take this.
Well, it is basically the same for all drivers using the gem shmem
helpers. So I see little reason why we should do that at virtio-gpu
level.
> Android GPU vendors have integrated this tracepoint to track gpu
> memory usage total(mapped into the gpu address space), which consists
> of below:
> (1) directly allocated via physical page allocator
> (2) imported external memory backed by dma-bufs
> (3) allocated exportable memory backed by dma-bufs
Hmm, the tracepoint doesn't track which of the three groups the memory
belongs to. Which I think is important, specifically group (2) because
that might already be accounted for by the exporting driver ...
> Our Android kernel team is leading the other side of effort to help
> remove the dma-bufs overlap(those mapped into a gpu device) as a joint
> effort, so that we can accurately explain the memory usage of the
> entire Android system.
I suspect once you figured that you'll notice that this little hack is
rather incomplete.
> For virtio-gpu, since that's used by our reference platform
> Cuttlefish(Cloud Android), we have to integrate the same tracepoint as
> well to enforce the use of this tracepoint and the eBPF stuff built on
> top to support runtime query of gpu memory on production devices. For
> virtio-gpu at this moment, we only want to track GEM allocations since
> PRIME import is currently not supported/used in Cuttlefish. That's all
> we are doing in this small patch.
take care,
Gerd
[PATCH v3 1/4] drm/qxl: use drmm_mode_config_init
Signed-off-by: Gerd Hoffmann
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 012bce0cdb65..38d6b596094d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1195,7 +1195,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
int i;
int ret;
- drm_mode_config_init(&qdev->ddev);
+ ret = drmm_mode_config_init(&qdev->ddev);
+ if (ret)
+ return ret;
ret = qxl_create_monitors_object(qdev);
if (ret)
@@ -1228,5 +1230,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(&qdev->ddev);
}
--
2.29.2
[PATCH v3 3/4] drm/qxl: release shadow on shutdown
In case we have a shadow surface on shutdown release
it so it doesn't leak.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 38d6b596094d..60331e31861a 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1229,5 +1229,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
+ if (qdev->dumb_shadow_bo) {
+ drm_gem_object_put(&qdev->dumb_shadow_bo->tbo.base);
+ qdev->dumb_shadow_bo = NULL;
+ }
qxl_destroy_monitors_object(qdev);
}
--
2.29.2
[PATCH v3 2/4] drm/qxl: unpin release objects
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(&bo->tbo); */ qxl_bo_unref(&bo); list_del(&entry->tv.head); kfree(entry); -- 2.29.2
[PATCH v3 4/4] drm/qxl: handle shadow in primary destroy
qxl_primary_atomic_disable must check whenever the framebuffer bo has a
shadow surface and in case it has check the shadow primary status.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 60331e31861a..f5ee8cd72b5b 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -562,6 +562,8 @@ static void qxl_primary_atomic_disable(struct drm_plane
*plane,
if (old_state->fb) {
struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
+ if (bo->shadow)
+ bo = bo->shadow;
if (bo->is_primary) {
qxl_io_destroy_primary(qdev);
bo->is_primary = false;
--
2.29.2
Re: [PATCH] drivers: gpu: drm: virtio: fix dependency of DRM_VIRTIO_GPU on VIRTIO
On Fri, Dec 04, 2020 at 02:12:21PM +0100, Enrico Weigelt, metux IT consult wrote: > VIRTIO itself has no dependencies and therefore can easily be just > select'ed, instead of depending on it. The current depends on causes > any others trying to select VIRTIO to fail like this: > >drivers/gpu/drm/Kconfig:74:error: recursive dependency detected! >drivers/gpu/drm/Kconfig:74:symbol DRM_KMS_HELPER is selected by > DRM_VIRTIO_GPU >drivers/gpu/drm/virtio/Kconfig:2: symbol DRM_VIRTIO_GPU depends on VIRTIO >drivers/virtio/Kconfig:2: symbol VIRTIO is selected by GPIO_VIRTIO >drivers/gpio/Kconfig:1618: symbol GPIO_VIRTIO depends on GPIOLIB >drivers/gpio/Kconfig:14: symbol GPIOLIB is selected by I2C_MUX_LTC4306 >drivers/i2c/muxes/Kconfig:47: symbol I2C_MUX_LTC4306 depends on I2C >drivers/i2c/Kconfig:8: symbol I2C is selected by FB_DDC >drivers/video/fbdev/Kconfig:63:symbol FB_DDC depends on FB >drivers/video/fbdev/Kconfig:12:symbol FB is selected by > DRM_KMS_FB_HELPER >drivers/gpu/drm/Kconfig:80:symbol DRM_KMS_FB_HELPER depends on > DRM_KMS_HELPER > > It seems that having both 'depends on' as well as 'select' on the same symbol > sends us into big trouble, and Kconfig can't break up the circular dependency > (note that in the tested configuration, neither I2C, FB or DRM are enabled at > all). Perhaps we could consider this a bug in Kconfig, but the trouble can > easily be circumvented by changing 'depends on' into 'select'. > > DRM_VIRTIO_GPU also depends on VIRTIO_MENU, so even after this change, that > option will only show up if the user already enabled virtio in the config. > > This change didn't cause any changes in the .config after menuconfig run, > so we should be completely safe here. > > Signed-off-by: Enrico Weigelt, metux IT consult Pushed to drm-misc-next. thanks, Gerd
Re: [PATCH -next] drm/virtio: Make virtgpu_dmabuf_ops with static keyword
On Sat, Nov 14, 2020 at 03:16:13PM +0800, Zou Wei wrote: > Fix the following sparse warning: > > ./virtgpu_prime.c:46:33: warning: symbol 'virtgpu_dmabuf_ops' was not > declared. Should it be static? Pushed to drm-misc-next. thanks, Gerd
Re: [PATCH] drm/qxl: replace idr_init() by idr_init_base()
On Fri, Nov 06, 2020 at 12:20:16AM +0530, Deepak R Varma wrote:
> idr_init() uses base 0 which is an invalid identifier for this driver.
> The idr_alloc for this driver uses 1 as start value for ID range. The
> new function idr_init_base allows IDR to set the ID lookup from base 1.
> This avoids all lookups that otherwise starts from 0 since 0 is always
> unused / available.
>
> References: commit 6ce711f27500 ("idr: Make 1-based IDRs more efficient")
>
> Signed-off-by: Deepak R Varma
Pushed to drm-misc-next.
thanks,
Gerd
Re: [PATCH] drm/virtio: use kvmalloc for large allocations
On Thu, Nov 05, 2020 at 04:00:54PM +0900, Sergey Senozhatsky wrote: > Hi, > > On (20/11/05 07:52), Gerd Hoffmann wrote: > > > - *ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry), > > > - GFP_KERNEL); > > > + *ents = kvmalloc_array(*nents, > > > +sizeof(struct virtio_gpu_mem_entry), > > > +GFP_KERNEL); > > > > Shouldn't that be balanced with a kvfree() elsewhere? > > I think it already is. ents pointer is assigned to vbuf->data_buf, > and free_vbuf() already uses kvfree(vbuf->data_buf) to free it. Ah, right, we needed that before elsewhere. Ok then, pushed to drm-misc-next. thanks, Gerd
Re: [PATCH] drm/virtio: use kvmalloc for large allocations
Hi, > - *ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry), > - GFP_KERNEL); > + *ents = kvmalloc_array(*nents, > +sizeof(struct virtio_gpu_mem_entry), > +GFP_KERNEL); Shouldn't that be balanced with a kvfree() elsewhere? take care, Gerd
Re: [PATCH v3 25/32] drm: kernel-doc: add description for a new function parameter
On Tue, Oct 27, 2020 at 10:51:29AM +0100, Mauro Carvalho Chehab wrote:
> As reported by "make htmldocs":
>
> ./drivers/gpu/drm/drm_prime.c:808: warning: Function parameter or
> member 'dev' not described in 'drm_prime_pages_to_sg'
>
> Add a description for the new parameter.
>
> Fixes: 707d561f77b5 ("drm: allow limiting the scatter list size.")
> Signed-off-by: Mauro Carvalho Chehab
Acked-by: Gerd Hoffmann
> ---
> drivers/gpu/drm/drm_prime.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index d6808f678db5..9f955f2010c2 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -794,6 +794,7 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops
> = {
>
> /**
> * drm_prime_pages_to_sg - converts a page array into an sg list
> + * @dev: DRM device
> * @pages: pointer to the array of page pointers to convert
> * @nr_pages: length of the page vector
> *
> --
> 2.26.2
>
[PATCH v2 4/4] drm/qxl: use qxl pin function
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(&bo->tbo);
+ qxl_bo_pin(bo);
*bo_ptr = bo;
return 0;
}
--
2.27.0
[PATCH v2 2/4] drm/qxl: release shadow on shutdown
In case we have a shadow surface on shutdown release
it so it doesn't leak.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 5bef8f121e54..1d9c51022be4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1220,5 +1220,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
+ if (qdev->dumb_shadow_bo) {
+ drm_gem_object_put(&qdev->dumb_shadow_bo->tbo.base);
+ qdev->dumb_shadow_bo = NULL;
+ }
qxl_destroy_monitors_object(qdev);
}
--
2.27.0
[PATCH v2 1/4] drm/qxl: use drmm_mode_config_init
Signed-off-by: Gerd Hoffmann
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 65de1f69af58..5bef8f121e54 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1186,7 +1186,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
int i;
int ret;
- drm_mode_config_init(&qdev->ddev);
+ ret = drmm_mode_config_init(&qdev->ddev);
+ if (ret)
+ return ret;
ret = qxl_create_monitors_object(qdev);
if (ret)
@@ -1219,5 +1221,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(&qdev->ddev);
}
--
2.27.0
[PATCH v2 3/4] drm/qxl: handle shadow in primary destroy
qxl_primary_atomic_disable must check whenever the framebuffer bo has a
shadow surface and in case it has check the shadow primary status.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 1d9c51022be4..d133e6c2aaf4 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -561,6 +561,8 @@ static void qxl_primary_atomic_disable(struct drm_plane
*plane,
if (old_state->fb) {
struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
+ if (bo->shadow)
+ bo = bo->shadow;
if (bo->is_primary) {
qxl_io_destroy_primary(qdev);
bo->is_primary = false;
--
2.27.0
Re: [PATCH -next] drm/qxl: simplify the return expression of qxl_plane_prepare_fb()
On Mon, Sep 21, 2020 at 09:10:22PM +0800, Qinglang Miao wrote: > Simplify the return expression. Pushed to drm-misc-next. thanks, Gerd
Re: [PATCH v2 0/3] KVM: x86: KVM_MEM_PCI_HOLE memory
Hi, > > We could probably wire up ecam (arm/virt style) for pcie support, once > > the acpi support for mictovm finally landed (we need acpi for that > > because otherwise the kernel wouldn't find the pcie bus). > > > > Question is whenever there is a good reason to do so. Why would someone > > prefer microvm with pcie support over q35? > > The usual reasons to use pcie apply to microvm just the same. > E.g.: pass through of pcie devices? Playground: https://git.kraxel.org/cgit/qemu/log/?h=sirius/microvm-usb Adds support for usb and pcie (use -machine microvm,usb=on,pcie=on to enable). Reuses the gpex used on arm/aarch64. Seems to work ok on a quick test. Not fully sure how to deal correctly with ioports. The gpex device has a mmio window for the io address space. Will that approach work on x86 too? Anyway, just not having a ioport range seems to be a valid configuation, so I've just disabled them for now ... take care, Gerd
Re: [PATCH v2 0/3] KVM: x86: KVM_MEM_PCI_HOLE memory
Hi, > I see a similar ~8k PCI hole reads with a -kernel boot w/ OVMF. All but 60 > of those are from pcibios_fixup_peer_bridges(), and all are from the kernel. pcibios_fixup_peer_bridges() looks at pcibios_last_bus, and that in turn seems to be set according to the mmconfig size (in arch/x86/pci/mmconfig-shared.c). So, maybe we just need to declare a smaller mmconfig window in the acpi tables, depending on the number of pci busses actually used ... > If all of the above is true, this can be handled by adding "pci=lastbus=0" ... so we don't need manual quirks like this? take care, Gerd
Re: [PATCH] drm/virtio: drop quirks handling
On Tue, Sep 08, 2020 at 10:57:21AM +0200, Daniel Vetter wrote: > On Tue, Sep 08, 2020 at 08:47:41AM +0200, Gerd Hoffmann wrote: > > These days dma ops can be overridden per device, and the virtio core > > "can be overridden" or "are"? Didn't happen yet, so scratch this one for now ... take care, Gerd
Re: [PATCH v4 1/1] drm: allow limiting the scatter list size.
> > > The comments I've found suggest very much not ... Or is that all very > > > old stuff only that no one cares about anymore? > > > > I think these days it is possible to override dma_ops per device, which > > in turn allows virtio to deal with the quirks without the rest of the > > kernel knowing about these details. > > > > I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just > > use the dma api path unconditionally and depend on virtio core having > > setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. > > The comment above vring_use_dma_api() suggests that this has not yet > happened, that's why I'm asking. Hmm, wading through the code, seems it indeed happen yet, even though my testing didn't show any issues. Probably pure luck because devices and cpus have the same memory view on x86. Guess I need to try this on ppc64 to see it actually failing ... So dropping the virtio_has_dma_quirk() checks isn't going to fly. Using dma_max_mapping_size() should be fine though. It might use a lower limit than needed for virtio, but it should not break things. take care, Gerd
[PATCH 3/3] drm/qxl: handle shadow in primary destroy
qxl_primary_atomic_disable must check whenever the framebuffer bo has a
shadow surface and in case it has check the shadow primary status.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 85dcb7fe56a9..3dcbb359e0f5 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -560,6 +560,8 @@ static void qxl_primary_atomic_disable(struct drm_plane
*plane,
if (old_state->fb) {
struct qxl_bo *bo = gem_to_qxl_bo(old_state->fb->obj[0]);
+ if (bo->shadow)
+ bo = bo->shadow;
if (bo->is_primary) {
qxl_io_destroy_primary(qdev);
bo->is_primary = false;
--
2.27.0
[PATCH 1/3] drm/qxl: use drmm_mode_config_init
Signed-off-by: Gerd Hoffmann
---
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(&qdev->ddev);
+ ret = drmm_mode_config_init(&qdev->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(&qdev->ddev);
}
--
2.27.0
[PATCH 2/3] drm/qxl: release shadow on shutdown
In case we have a shadow surface on shutdown release
it so it doesn't leak.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/qxl/qxl_display.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/gpu/drm/qxl/qxl_display.c
b/drivers/gpu/drm/qxl/qxl_display.c
index 4be04eaf7f37..85dcb7fe56a9 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -1224,5 +1224,9 @@ int qxl_modeset_init(struct qxl_device *qdev)
void qxl_modeset_fini(struct qxl_device *qdev)
{
+ if (qdev->dumb_shadow_bo) {
+ drm_gem_object_put(&qdev->dumb_shadow_bo->tbo.base);
+ qdev->dumb_shadow_bo = NULL;
+ }
qxl_destroy_monitors_object(qdev);
}
--
2.27.0
[PATCH 2/3] drm/virtio: return virtio_gpu_queue errors
In case queuing virtio commands fails (can happen when
the device got unplugged) pass up the error.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 36 +++--
1 file changed, 19 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index c93c2db35aaf..b1884e6e242c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -320,13 +320,13 @@ static struct sg_table *vmalloc_to_sgt(char *data,
uint32_t size, int *sg_ents)
return sgt;
}
-static void virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf,
- struct virtio_gpu_fence *fence,
- int elemcnt,
- struct scatterlist **sgs,
- int outcnt,
- int incnt)
+static int virtio_gpu_queue_ctrl_sgs(struct virtio_gpu_device *vgdev,
+struct virtio_gpu_vbuffer *vbuf,
+struct virtio_gpu_fence *fence,
+int elemcnt,
+struct scatterlist **sgs,
+int outcnt,
+int incnt)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
int ret, idx;
@@ -335,7 +335,7 @@ static void virtio_gpu_queue_ctrl_sgs(struct
virtio_gpu_device *vgdev,
if (fence && vbuf->objs)
virtio_gpu_array_unlock_resv(vbuf->objs);
free_vbuf(vgdev, vbuf);
- return;
+ return -1;
}
if (vgdev->has_indirect)
@@ -373,15 +373,16 @@ static void virtio_gpu_queue_ctrl_sgs(struct
virtio_gpu_device *vgdev,
spin_unlock(&vgdev->ctrlq.qlock);
drm_dev_exit(idx);
+ return 0;
}
-static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device
*vgdev,
- struct virtio_gpu_vbuffer *vbuf,
- struct virtio_gpu_fence *fence)
+static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_vbuffer *vbuf,
+ struct virtio_gpu_fence *fence)
{
struct scatterlist *sgs[3], vcmd, vout, vresp;
struct sg_table *sgt = NULL;
- int elemcnt = 0, outcnt = 0, incnt = 0;
+ int elemcnt = 0, outcnt = 0, incnt = 0, ret;
/* set up vcmd */
sg_init_one(&vcmd, vbuf->buf, vbuf->size);
@@ -398,7 +399,7 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
if (!sgt) {
if (fence && vbuf->objs)
virtio_gpu_array_unlock_resv(vbuf->objs);
- return;
+ return -1;
}
elemcnt += sg_ents;
@@ -419,13 +420,14 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct
virtio_gpu_device *vgdev,
incnt++;
}
- virtio_gpu_queue_ctrl_sgs(vgdev, vbuf, fence, elemcnt, sgs, outcnt,
- incnt);
+ ret = virtio_gpu_queue_ctrl_sgs(vgdev, vbuf, fence, elemcnt, sgs,
outcnt,
+ incnt);
if (sgt) {
sg_free_table(sgt);
kfree(sgt);
}
+ return ret;
}
void virtio_gpu_notify(struct virtio_gpu_device *vgdev)
@@ -444,10 +446,10 @@ void virtio_gpu_notify(struct virtio_gpu_device *vgdev)
virtqueue_notify(vgdev->ctrlq.vq);
}
-static void virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
+static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf)
{
- virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL);
+ return virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL);
}
static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
--
2.27.0
[PATCH 3/3] drm/virtio: add virtio_gpu_cmd_unref_resource error handling
Usually we wait for the host to complete the unref request, then cleanup
the guest-side state of the object in the completion callback. When
submitting the unref command failed the completion callback will not be
called though, so cleanup right away.
Fixes a WARN on stale mm entries on driver shutdown.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index b1884e6e242c..4d2325bf4aed 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -536,6 +536,7 @@ void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device
*vgdev,
{
struct virtio_gpu_resource_unref *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
+ int ret;
cmd_p = virtio_gpu_alloc_cmd_cb(vgdev, &vbuf, sizeof(*cmd_p),
virtio_gpu_cmd_unref_cb);
@@ -545,7 +546,9 @@ void virtio_gpu_cmd_unref_resource(struct virtio_gpu_device
*vgdev,
cmd_p->resource_id = cpu_to_le32(bo->hw_res_handle);
vbuf->resp_cb_data = bo;
- virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
+ ret = virtio_gpu_queue_ctrl_buffer(vgdev, vbuf);
+ if (ret < 0)
+ virtio_gpu_cleanup_object(bo);
}
void virtio_gpu_cmd_set_scanout(struct virtio_gpu_device *vgdev,
--
2.27.0
[PATCH 1/3] drm/virtio: use drmm_mode_config_init
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/virtio/virtgpu_drv.h | 2 +-
drivers/gpu/drm/virtio/virtgpu_display.c | 11 +++
drivers/gpu/drm/virtio/virtgpu_kms.c | 6 +-
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h
b/drivers/gpu/drm/virtio/virtgpu_drv.h
index a52b7a39f286..55c34b4fc3e9 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -352,7 +352,7 @@ virtio_gpu_cmd_resource_assign_uuid(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_object_array *objs);
/* virtgpu_display.c */
-void virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev);
+int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev);
void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev);
/* virtgpu_plane.c */
diff --git a/drivers/gpu/drm/virtio/virtgpu_display.c
b/drivers/gpu/drm/virtio/virtgpu_display.c
index effea07abe62..f84b7e61311b 100644
--- a/drivers/gpu/drm/virtio/virtgpu_display.c
+++ b/drivers/gpu/drm/virtio/virtgpu_display.c
@@ -325,11 +325,14 @@ static const struct drm_mode_config_funcs
virtio_gpu_mode_funcs = {
.atomic_commit = drm_atomic_helper_commit,
};
-void virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev)
+int virtio_gpu_modeset_init(struct virtio_gpu_device *vgdev)
{
- int i;
+ int i, ret;
+
+ ret = drmm_mode_config_init(vgdev->ddev);
+ if (ret)
+ return ret;
- drm_mode_config_init(vgdev->ddev);
vgdev->ddev->mode_config.quirk_addfb_prefer_host_byte_order = true;
vgdev->ddev->mode_config.funcs = &virtio_gpu_mode_funcs;
@@ -343,6 +346,7 @@ void virtio_gpu_modeset_init(struct virtio_gpu_device
*vgdev)
vgdev_output_init(vgdev, i);
drm_mode_config_reset(vgdev->ddev);
+ return 0;
}
void virtio_gpu_modeset_fini(struct virtio_gpu_device *vgdev)
@@ -351,5 +355,4 @@ void virtio_gpu_modeset_fini(struct virtio_gpu_device
*vgdev)
for (i = 0 ; i < vgdev->num_scanouts; ++i)
kfree(vgdev->outputs[i].edid);
- drm_mode_config_cleanup(vgdev->ddev);
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 75d0dc2f6d28..6ea74a99d8ad 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -184,7 +184,11 @@ int virtio_gpu_init(struct drm_device *dev)
num_capsets, &num_capsets);
DRM_INFO("number of cap sets: %d\n", num_capsets);
- virtio_gpu_modeset_init(vgdev);
+ ret = virtio_gpu_modeset_init(vgdev);
+ if (ret) {
+ DRM_ERROR("modeset init failed\n");
+ goto err_scanouts;
+ }
virtio_device_ready(vgdev->vdev);
--
2.27.0
[PATCH] drm/virtio: drop quirks handling
These days dma ops can be overridden per device, and the virtio core
uses that to handle the dma quirks transparently for the rest of the
kernel. So we can drop the virtio_has_dma_quirk() checks, just use
the dma api unconditionally and depend on the virtio core having setup
dma_ops as needed.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/virtio/virtgpu_object.c | 19 ++-
drivers/gpu/drm/virtio/virtgpu_vq.c | 16 ++--
2 files changed, 12 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_object.c
b/drivers/gpu/drm/virtio/virtgpu_object.c
index 729f98ad7c02..9c35ce64ff9e 100644
--- a/drivers/gpu/drm/virtio/virtgpu_object.c
+++ b/drivers/gpu/drm/virtio/virtgpu_object.c
@@ -141,7 +141,6 @@ static int virtio_gpu_object_shmem_init(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_mem_entry **ents,
unsigned int *nents)
{
- bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev);
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
struct scatterlist *sg;
int si, ret;
@@ -162,15 +161,11 @@ static int virtio_gpu_object_shmem_init(struct
virtio_gpu_device *vgdev,
return -EINVAL;
}
- if (use_dma_api) {
- shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent,
- shmem->pages->sgl,
- shmem->pages->nents,
- DMA_TO_DEVICE);
- *nents = shmem->mapped;
- } else {
- *nents = shmem->pages->nents;
- }
+ shmem->mapped = dma_map_sg(vgdev->vdev->dev.parent,
+ shmem->pages->sgl,
+ shmem->pages->nents,
+ DMA_TO_DEVICE);
+ *nents = shmem->mapped;
*ents = kmalloc_array(*nents, sizeof(struct virtio_gpu_mem_entry),
GFP_KERNEL);
@@ -180,9 +175,7 @@ static int virtio_gpu_object_shmem_init(struct
virtio_gpu_device *vgdev,
}
for_each_sg(shmem->pages->sgl, sg, *nents, si) {
- (*ents)[si].addr = cpu_to_le64(use_dma_api
- ? sg_dma_address(sg)
- : sg_phys(sg));
+ (*ents)[si].addr = cpu_to_le64(sg_dma_address(sg));
(*ents)[si].length = cpu_to_le32(sg->length);
(*ents)[si].padding = 0;
}
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c
b/drivers/gpu/drm/virtio/virtgpu_vq.c
index c93c2db35aaf..1c1d2834547d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -599,13 +599,11 @@ void virtio_gpu_cmd_transfer_to_host_2d(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]);
struct virtio_gpu_transfer_to_host_2d *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
- bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev);
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
- if (use_dma_api)
- dma_sync_sg_for_device(vgdev->vdev->dev.parent,
- shmem->pages->sgl, shmem->pages->nents,
- DMA_TO_DEVICE);
+ dma_sync_sg_for_device(vgdev->vdev->dev.parent,
+ shmem->pages->sgl, shmem->pages->nents,
+ DMA_TO_DEVICE);
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
@@ -1015,13 +1013,11 @@ void virtio_gpu_cmd_transfer_to_host_3d(struct
virtio_gpu_device *vgdev,
struct virtio_gpu_object *bo = gem_to_virtio_gpu_obj(objs->objs[0]);
struct virtio_gpu_transfer_host_3d *cmd_p;
struct virtio_gpu_vbuffer *vbuf;
- bool use_dma_api = !virtio_has_dma_quirk(vgdev->vdev);
struct virtio_gpu_object_shmem *shmem = to_virtio_gpu_shmem(bo);
- if (use_dma_api)
- dma_sync_sg_for_device(vgdev->vdev->dev.parent,
- shmem->pages->sgl, shmem->pages->nents,
- DMA_TO_DEVICE);
+ dma_sync_sg_for_device(vgdev->vdev->dev.parent,
+ shmem->pages->sgl, shmem->pages->nents,
+ DMA_TO_DEVICE);
cmd_p = virtio_gpu_alloc_cmd(vgdev, &vbuf, sizeof(*cmd_p));
memset(cmd_p, 0, sizeof(*cmd_p));
--
2.27.0
Re: [PATCH v4 1/1] drm: allow limiting the scatter list size.
On Mon, Sep 07, 2020 at 03:53:02PM +0200, Daniel Vetter wrote: > On Mon, Sep 7, 2020 at 1:24 PM Gerd Hoffmann wrote: > > > > Add drm_device argument to drm_prime_pages_to_sg(), so we can > > call dma_max_mapping_size() to figure the segment size limit > > and call into __sg_alloc_table_from_pages() with the correct > > limit. > > > > This fixes virtio-gpu with sev. Possibly it'll fix other bugs > > too given that drm seems to totaly ignore segment size limits > > so far ... > > > > v2: place max_segment in drm driver not gem object. > > v3: move max_segment next to the other gem fields. > > v4: just use dma_max_mapping_size(). > > > > Signed-off-by: Gerd Hoffmann > > Uh, are you sure this works in all cases for virtio? Sure, I've tested it ;) > The comments I've found suggest very much not ... Or is that all very > old stuff only that no one cares about anymore? I think these days it is possible to override dma_ops per device, which in turn allows virtio to deal with the quirks without the rest of the kernel knowing about these details. I also think virtio-gpu can drop the virtio_has_dma_quirk() checks, just use the dma api path unconditionally and depend on virtio core having setup dma_ops in a way that it JustWorks[tm]. I'll look into that next. take care, Gerd
[PATCH v4 1/1] drm: allow limiting the scatter list size.
Add drm_device argument to drm_prime_pages_to_sg(), so we can
call dma_max_mapping_size() to figure the segment size limit
and call into __sg_alloc_table_from_pages() with the correct
limit.
This fixes virtio-gpu with sev. Possibly it'll fix other bugs
too given that drm seems to totaly ignore segment size limits
so far ...
v2: place max_segment in drm driver not gem object.
v3: move max_segment next to the other gem fields.
v4: just use dma_max_mapping_size().
Signed-off-by: Gerd Hoffmann
---
include/drm/drm_prime.h | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 ++-
drivers/gpu/drm/drm_gem_shmem_helper.c | 2 +-
drivers/gpu/drm/drm_prime.c | 13 ++---
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 3 ++-
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 2 +-
drivers/gpu/drm/msm/msm_gem.c | 2 +-
drivers/gpu/drm/msm/msm_gem_prime.c | 2 +-
drivers/gpu/drm/nouveau/nouveau_prime.c | 2 +-
drivers/gpu/drm/radeon/radeon_prime.c | 2 +-
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 5 +++--
drivers/gpu/drm/tegra/gem.c | 2 +-
drivers/gpu/drm/vgem/vgem_drv.c | 2 +-
drivers/gpu/drm/xen/xen_drm_front_gem.c | 3 ++-
14 files changed, 29 insertions(+), 17 deletions(-)
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 9af7422b44cf..bf141e74a1c2 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -88,7 +88,8 @@ void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void
*vaddr);
int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma);
-struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages);
+struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev,
+ struct page **pages, unsigned int
nr_pages);
struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
int flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 519ce4427fce..d7050ab95946 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -302,7 +302,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
switch (bo->tbo.mem.mem_type) {
case TTM_PL_TT:
- sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages,
+ sgt = drm_prime_pages_to_sg(obj->dev,
+ bo->tbo.ttm->pages,
bo->tbo.num_pages);
if (IS_ERR(sgt))
return sgt;
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 4b7cfbac4daa..0a952f27c184 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -656,7 +656,7 @@ struct sg_table *drm_gem_shmem_get_sg_table(struct
drm_gem_object *obj)
WARN_ON(shmem->base.import_attach);
- return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT);
+ return drm_prime_pages_to_sg(obj->dev, shmem->pages, obj->size >>
PAGE_SHIFT);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 1693aa7c14b5..8a6a3c99b7d8 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -802,9 +802,11 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =
{
*
* This is useful for implementing &drm_gem_object_funcs.get_sg_table.
*/
-struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages)
+struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev,
+ struct page **pages, unsigned int
nr_pages)
{
struct sg_table *sg = NULL;
+ size_t max_segment = 0;
int ret;
sg = kmalloc(sizeof(struct sg_table), GFP_KERNEL);
@@ -813,8 +815,13 @@ struct sg_table *drm_prime_pages_to_sg(struct page
**pages, unsigned int nr_page
goto out;
}
- ret = sg_alloc_table_from_pages(sg, pages, nr_pages, 0,
- nr_pages << PAGE_SHIFT, GFP_KERNEL);
+ if (dev)
+ max_segment = dma_max_mapping_size(dev->dev);
+ if (max_segment == 0 || max_segment > SCATTERLIST_MAX_SEGMENT)
+ max_segment = SCATTERLIST_MAX_SEGMENT;
+ ret = __sg_alloc_table_from_pages(sg, pages, nr_pages, 0,
+ nr_pages << PAGE_SHIFT,
+ max_segment, GFP_KERNEL);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.c
b/drivers/gpu/drm/etnaviv/etnav
Re: [PATCH v2 1/2] drm: allow limiting the scatter list size.
> > + /**
> > +* @max_segment:
> > +*
> > +* Max size for scatter list segments. When unset the default
> > +* (SCATTERLIST_MAX_SEGMENT) is used.
> > +*/
> > + size_t max_segment;
>
> Is there no better place for this then "at the bottom"? drm_device is a
> huge structure, piling stuff up randomly doesn't make it better :-)
Moved next to the other gem fields for now (v3 posted).
> I think ideally we'd have a gem substruct like we have on the modeset side
> at least.
Phew, that'll be quite some churn in the tree. And there aren't that many
gem-related fields in struct drm_device.
So you are looking for something like below (header changes only)?
take care,
Gerd
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index c455ef404ca6..950167ede98a 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -299,22 +299,8 @@ struct drm_device {
/** @mode_config: Current mode config */
struct drm_mode_config mode_config;
- /** @object_name_lock: GEM information */
- struct mutex object_name_lock;
-
- /** @object_name_idr: GEM information */
- struct idr object_name_idr;
-
- /** @vma_offset_manager: GEM information */
- struct drm_vma_offset_manager *vma_offset_manager;
-
- /**
-* @max_segment:
-*
-* Max size for scatter list segments for GEM objects. When
-* unset the default (SCATTERLIST_MAX_SEGMENT) is used.
-*/
- size_t max_segment;
+ /** @gem_config: Current GEM config */
+ struct drm_gem_config gem_config;
/** @vram_mm: VRAM MM memory manager */
struct drm_vram_mm *vram_mm;
diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h
index 337a48321705..74129fb29fb8 100644
--- a/include/drm/drm_gem.h
+++ b/include/drm/drm_gem.h
@@ -39,6 +39,25 @@
#include
+struct drm_gem_config {
+ /** @object_name_lock: GEM information */
+ struct mutex object_name_lock;
+
+ /** @object_name_idr: GEM information */
+ struct idr object_name_idr;
+
+ /** @vma_offset_manager: GEM information */
+ struct drm_vma_offset_manager *vma_offset_manager;
+
+ /**
+* @max_segment:
+*
+* Max size for scatter list segments for GEM objects. When
+* unset the default (SCATTERLIST_MAX_SEGMENT) is used.
+*/
+ size_t max_segment;
+};
+
struct drm_gem_object;
/**
[PATCH v3 1/2] drm: allow limiting the scatter list size.
Add max_segment argument to drm_prime_pages_to_sg(). When set pass it
through to the __sg_alloc_table_from_pages() call, otherwise use
SCATTERLIST_MAX_SEGMENT.
Also add max_segment field to drm driver and pass it to
drm_prime_pages_to_sg() calls in drivers and helpers.
v2: place max_segment in drm driver not gem object.
v3: move max_segment next to the other gem fields.
Signed-off-by: Gerd Hoffmann
---
include/drm/drm_device.h| 8
include/drm/drm_prime.h | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 ++-
drivers/gpu/drm/drm_gem_shmem_helper.c | 3 ++-
drivers/gpu/drm/drm_prime.c | 10 +++---
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 3 ++-
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 3 ++-
drivers/gpu/drm/msm/msm_gem.c | 3 ++-
drivers/gpu/drm/msm/msm_gem_prime.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_prime.c | 3 ++-
drivers/gpu/drm/radeon/radeon_prime.c | 3 ++-
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 6 --
drivers/gpu/drm/tegra/gem.c | 3 ++-
drivers/gpu/drm/vgem/vgem_drv.c | 3 ++-
drivers/gpu/drm/xen/xen_drm_front_gem.c | 3 ++-
15 files changed, 43 insertions(+), 17 deletions(-)
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index f4f68e7a9149..c455ef404ca6 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -308,6 +308,14 @@ struct drm_device {
/** @vma_offset_manager: GEM information */
struct drm_vma_offset_manager *vma_offset_manager;
+ /**
+* @max_segment:
+*
+* Max size for scatter list segments for GEM objects. When
+* unset the default (SCATTERLIST_MAX_SEGMENT) is used.
+*/
+ size_t max_segment;
+
/** @vram_mm: VRAM MM memory manager */
struct drm_vram_mm *vram_mm;
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 9af7422b44cf..2c3689435cb4 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -88,7 +88,8 @@ void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void
*vaddr);
int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma);
-struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages);
+struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages,
+ size_t max_segment);
struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
int flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 519ce4427fce..8f6a647757e7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -303,7 +303,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
switch (bo->tbo.mem.mem_type) {
case TTM_PL_TT:
sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages,
- bo->tbo.num_pages);
+ bo->tbo.num_pages,
+ obj->dev->max_segment);
if (IS_ERR(sgt))
return sgt;
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 4b7cfbac4daa..8f47b41b0b2f 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -656,7 +656,8 @@ struct sg_table *drm_gem_shmem_get_sg_table(struct
drm_gem_object *obj)
WARN_ON(shmem->base.import_attach);
- return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT);
+ return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT,
+obj->dev->max_segment);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 1693aa7c14b5..27c783fd6633 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -802,7 +802,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =
{
*
* This is useful for implementing &drm_gem_object_funcs.get_sg_table.
*/
-struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages)
+struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages,
+ size_t max_segment)
{
struct sg_table *sg = NULL;
int ret;
@@ -813,8 +814,11 @@ struct sg_table *drm_prime_pages_to_sg(struct page
**pages, unsigned int nr_page
goto out;
}
- ret = sg_alloc_table_from_pages(sg, pages, nr_pages, 0,
- nr_pages << PAGE
[PATCH v3 2/2] drm/virtio: set max_segment
When initializing call virtio_max_dma_size() to figure the scatter list
limit. Needed to make virtio-gpu work properly with SEV.
v2: place max_segment in drm driver not gem object.
Signed-off-by: Gerd Hoffmann
---
drivers/gpu/drm/virtio/virtgpu_kms.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c
b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 75d0dc2f6d28..151471acdfcf 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -167,6 +167,7 @@ int virtio_gpu_init(struct drm_device *dev)
DRM_ERROR("failed to alloc vbufs\n");
goto err_vbufs;
}
+ dev->max_segment = virtio_max_dma_size(vgdev->vdev);
/* get display info */
virtio_cread_le(vgdev->vdev, struct virtio_gpu_config,
--
2.27.0
Re: [PATCH v2 0/3] KVM: x86: KVM_MEM_PCI_HOLE memory
Hi, > Unless I'm mistaken, microvm doesn't even support PCI, does it? Correct, no pci support right now. We could probably wire up ecam (arm/virt style) for pcie support, once the acpi support for mictovm finally landed (we need acpi for that because otherwise the kernel wouldn't find the pcie bus). Question is whenever there is a good reason to do so. Why would someone prefer microvm with pcie support over q35? > If all of the above is true, this can be handled by adding "pci=lastbus=0" > as a guest kernel param to override its scanning of buses. And couldn't > that be done by QEMU's microvm_fix_kernel_cmdline() to make it transparent > to the end user? microvm_fix_kernel_cmdline() is a hack, not a solution. Beside that I doubt this has much of an effect on microvm because it doesn't support pcie in the first place. take care, Gerd
Re: [PATCH v2 0/3] KVM: x86: KVM_MEM_PCI_HOLE memory
Hi, > attached. Though those should be far less than 8000+, and those should also > be > pio rather than mmio. Well, seabios 1.14 (qemu 5.1) added mmio support (to speed up boot a little bit, mmio is one and pio is two vmexits). Depends on q35 obviously, and a few pio accesses remain b/c seabios has to first setup mmconfig. take care, Gerd
Re: [PATCH 1/2] drm/virtio: fix unblank
On Mon, Aug 24, 2020 at 09:24:40AM +0200, Jiri Slaby wrote:
> On 18. 08. 20, 9:25, Gerd Hoffmann wrote:
> > When going through a disable/enable cycle without changing the
> > framebuffer the optimization added by commit 3954ff10e06e ("drm/virtio:
> > skip set_scanout if framebuffer didn't change") causes the screen stay
> > blank. Add a bool to force an update to fix that.
> >
> > v2: use drm_atomic_crtc_needs_modeset() (Daniel).
> >
> > Cc: 1882...@bugs.launchpad.net
> > Fixes: 3954ff10e06e ("drm/virtio: skip set_scanout if framebuffer didn't
> > change")
> > Signed-off-by: Gerd Hoffmann
>
> Tested-by: Jiri Slaby
Ping. Need an ack or an review to merge this.
thanks,
Gerd
Re: [PATCH] drm: virtio: fix kconfig dependency warning
Hi, > config DRM_VIRTIO_GPU > tristate "Virtio GPU driver" > - depends on DRM && VIRTIO && MMU > + depends on DRM && VIRTIO_MENU && MMU Shouldn't this depend on both VIRTIO and VIRTIO_MENU, simliar to the other virtio drivers? take care, Gerd
Re: [PATCH v3 30/38] virtio_input: convert to LE accessors
On Wed, Aug 05, 2020 at 09:44:36AM -0400, Michael S. Tsirkin wrote: > Virtio input is modern-only. Use LE accessors for config space. > > Signed-off-by: Michael S. Tsirkin Reviewed-by: Gerd Hoffmann
[PATCH v2 2/2] drm/virtio: set max_segment
When initializing call virtio_max_dma_size() to figure the scatter list limit. Needed to make virtio-gpu work properly with SEV. v2: place max_segment in drm driver not gem object. Signed-off-by: Gerd Hoffmann --- drivers/gpu/drm/virtio/virtgpu_kms.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c index bf060c69850f..5a4364c00fae 100644 --- a/drivers/gpu/drm/virtio/virtgpu_kms.c +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c @@ -114,6 +114,7 @@ int virtio_gpu_init(struct drm_device *dev) vgdev->ddev = dev; dev->dev_private = vgdev; + dev->max_segment = virtio_max_dma_size(vgdev->vdev); vgdev->vdev = dev_to_virtio(dev->dev); vgdev->dev = dev->dev; -- 2.18.4
[PATCH v2 1/2] drm: allow limiting the scatter list size.
Add max_segment argument to drm_prime_pages_to_sg(). When set pass it
through to the __sg_alloc_table_from_pages() call, otherwise use
SCATTERLIST_MAX_SEGMENT.
Also add max_segment field to drm driver and pass it to
drm_prime_pages_to_sg() calls in drivers and helpers.
v2: place max_segment in drm driver not gem object.
Signed-off-by: Gerd Hoffmann
---
include/drm/drm_device.h| 8
include/drm/drm_prime.h | 3 ++-
drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 ++-
drivers/gpu/drm/drm_gem_shmem_helper.c | 3 ++-
drivers/gpu/drm/drm_prime.c | 10 +++---
drivers/gpu/drm/etnaviv/etnaviv_gem.c | 3 ++-
drivers/gpu/drm/etnaviv/etnaviv_gem_prime.c | 3 ++-
drivers/gpu/drm/msm/msm_gem.c | 3 ++-
drivers/gpu/drm/msm/msm_gem_prime.c | 3 ++-
drivers/gpu/drm/nouveau/nouveau_prime.c | 3 ++-
drivers/gpu/drm/radeon/radeon_prime.c | 3 ++-
drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 6 --
drivers/gpu/drm/tegra/gem.c | 3 ++-
drivers/gpu/drm/vgem/vgem_drv.c | 3 ++-
drivers/gpu/drm/xen/xen_drm_front_gem.c | 3 ++-
15 files changed, 43 insertions(+), 17 deletions(-)
diff --git a/include/drm/drm_device.h b/include/drm/drm_device.h
index 0988351d743c..47cb547a8115 100644
--- a/include/drm/drm_device.h
+++ b/include/drm/drm_device.h
@@ -329,6 +329,14 @@ struct drm_device {
*/
struct drm_fb_helper *fb_helper;
+ /**
+* @max_segment:
+*
+* Max size for scatter list segments. When unset the default
+* (SCATTERLIST_MAX_SEGMENT) is used.
+*/
+ size_t max_segment;
+
/* Everything below here is for legacy driver, never use! */
/* private: */
#if IS_ENABLED(CONFIG_DRM_LEGACY)
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index 9af7422b44cf..2c3689435cb4 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -88,7 +88,8 @@ void drm_gem_dmabuf_vunmap(struct dma_buf *dma_buf, void
*vaddr);
int drm_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma);
int drm_gem_dmabuf_mmap(struct dma_buf *dma_buf, struct vm_area_struct *vma);
-struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages);
+struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages,
+ size_t max_segment);
struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
int flags);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 519ce4427fce..8f6a647757e7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -303,7 +303,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct
dma_buf_attachment *attach,
switch (bo->tbo.mem.mem_type) {
case TTM_PL_TT:
sgt = drm_prime_pages_to_sg(bo->tbo.ttm->pages,
- bo->tbo.num_pages);
+ bo->tbo.num_pages,
+ obj->dev->max_segment);
if (IS_ERR(sgt))
return sgt;
diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c
b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 4b7cfbac4daa..8f47b41b0b2f 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -656,7 +656,8 @@ struct sg_table *drm_gem_shmem_get_sg_table(struct
drm_gem_object *obj)
WARN_ON(shmem->base.import_attach);
- return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT);
+ return drm_prime_pages_to_sg(shmem->pages, obj->size >> PAGE_SHIFT,
+obj->dev->max_segment);
}
EXPORT_SYMBOL_GPL(drm_gem_shmem_get_sg_table);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 1693aa7c14b5..27c783fd6633 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -802,7 +802,8 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops =
{
*
* This is useful for implementing &drm_gem_object_funcs.get_sg_table.
*/
-struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages)
+struct sg_table *drm_prime_pages_to_sg(struct page **pages, unsigned int
nr_pages,
+ size_t max_segment)
{
struct sg_table *sg = NULL;
int ret;
@@ -813,8 +814,11 @@ struct sg_table *drm_prime_pages_to_sg(struct page
**pages, unsigned int nr_page
goto out;
}
- ret = sg_alloc_table_from_pages(sg, pages, nr_pages, 0,
- nr_pages << PAGE_SHIFT, GFP_KERNEL);
+ if (max_segment == 0 || max_segment > SCATTERLIST
