* [PATCH v2 1/4] drm/qxl: use drmm_mode_config_init
[not found] <20200929095115.24430-1-kraxel@redhat.com>
@ 2020-09-29 9:51 ` Gerd Hoffmann
2020-09-29 9:51 ` [PATCH v2 2/4] drm/qxl: release shadow on shutdown Gerd Hoffmann
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2020-09-29 9:51 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, Daniel Vetter, Dave Airlie, David Airlie,
Daniel Vetter, open list:DRM DRIVER FOR QXL VIRTUAL GPU,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, open list
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/4] drm/qxl: release shadow on shutdown
[not found] <20200929095115.24430-1-kraxel@redhat.com>
2020-09-29 9:51 ` [PATCH v2 1/4] drm/qxl: use drmm_mode_config_init Gerd Hoffmann
@ 2020-09-29 9:51 ` Gerd Hoffmann
2020-09-29 9:51 ` [PATCH v2 3/4] drm/qxl: handle shadow in primary destroy Gerd Hoffmann
2020-09-29 9:51 ` [PATCH v2 4/4] drm/qxl: use qxl pin function Gerd Hoffmann
3 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2020-09-29 9:51 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, Dave Airlie, David Airlie, Daniel Vetter,
open list:DRM DRIVER FOR QXL VIRTUAL GPU,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, open list
In case we have a shadow surface on shutdown release
it so it doesn't leak.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 3/4] drm/qxl: handle shadow in primary destroy
[not found] <20200929095115.24430-1-kraxel@redhat.com>
2020-09-29 9:51 ` [PATCH v2 1/4] drm/qxl: use drmm_mode_config_init Gerd Hoffmann
2020-09-29 9:51 ` [PATCH v2 2/4] drm/qxl: release shadow on shutdown Gerd Hoffmann
@ 2020-09-29 9:51 ` Gerd Hoffmann
2020-09-29 9:51 ` [PATCH v2 4/4] drm/qxl: use qxl pin function Gerd Hoffmann
3 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2020-09-29 9:51 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, Dave Airlie, David Airlie, Daniel Vetter,
open list:DRM DRIVER FOR QXL VIRTUAL GPU,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, open list
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 <kraxel@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 4/4] drm/qxl: use qxl pin function
[not found] <20200929095115.24430-1-kraxel@redhat.com>
` (2 preceding siblings ...)
2020-09-29 9:51 ` [PATCH v2 3/4] drm/qxl: handle shadow in primary destroy Gerd Hoffmann
@ 2020-09-29 9:51 ` Gerd Hoffmann
2020-09-29 10:53 ` Daniel Vetter
3 siblings, 1 reply; 6+ messages in thread
From: Gerd Hoffmann @ 2020-09-29 9:51 UTC (permalink / raw)
To: dri-devel
Cc: Gerd Hoffmann, Dave Airlie, David Airlie, Daniel Vetter,
Huang Rui, Christian König,
open list:DRM DRIVER FOR QXL VIRTUAL GPU,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, open list
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 <kraxel@redhat.com>
---
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 4/4] drm/qxl: use qxl pin function
2020-09-29 9:51 ` [PATCH v2 4/4] drm/qxl: use qxl pin function Gerd Hoffmann
@ 2020-09-29 10:53 ` Daniel Vetter
2020-09-29 11:24 ` Christian König
0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2020-09-29 10:53 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: dri-devel, Dave Airlie, David Airlie, Daniel Vetter, Huang Rui,
Christian König, open list:DRM DRIVER FOR QXL VIRTUAL GPU,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, open list
On Tue, Sep 29, 2020 at 11:51:15AM +0200, Gerd Hoffmann wrote:
> Otherwise ttm throws a WARN because we try to pin without a reservation.
>
> Fixes: 9d36d4320462 ("drm/qxl: switch over to the new pin interface")
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> 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);
I think this is now after ttm_bo_init, and at that point the object is
visible to lru users and everything. So I do think you need to grab locks
here instead of just incrementing the pin count alone.
It's also I think a bit racy, since ttm_bo_init drops the lock, so someone
might have snuck in and evicted the object already.
I think what you need is to call ttm_bo_init_reserved, then ttm_bo_pin,
then ttm_bo_unreserve, all explicitly.
-Daniel
> *bo_ptr = bo;
> return 0;
> }
> --
> 2.27.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2 4/4] drm/qxl: use qxl pin function
2020-09-29 10:53 ` Daniel Vetter
@ 2020-09-29 11:24 ` Christian König
0 siblings, 0 replies; 6+ messages in thread
From: Christian König @ 2020-09-29 11:24 UTC (permalink / raw)
To: Gerd Hoffmann, dri-devel, Dave Airlie, David Airlie, Huang Rui,
open list:DRM DRIVER FOR QXL VIRTUAL GPU,
open list:DRM DRIVER FOR QXL VIRTUAL GPU, open list
Am 29.09.20 um 12:53 schrieb Daniel Vetter:
> On Tue, Sep 29, 2020 at 11:51:15AM +0200, Gerd Hoffmann wrote:
>> Otherwise ttm throws a WARN because we try to pin without a reservation.
>>
>> Fixes: 9d36d4320462 ("drm/qxl: switch over to the new pin interface")
>> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
>> ---
>> 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);
> I think this is now after ttm_bo_init, and at that point the object is
> visible to lru users and everything. So I do think you need to grab locks
> here instead of just incrementing the pin count alone.
>
> It's also I think a bit racy, since ttm_bo_init drops the lock, so someone
> might have snuck in and evicted the object already.
>
> I think what you need is to call ttm_bo_init_reserved, then ttm_bo_pin,
> then ttm_bo_unreserve, all explicitly.
Ah, yes Daniel is right. I thought I've fixed that up, but looks like I
only did that for VMWGFX.
Sorry for the noise, fix to correctly address this is underway.
Regards,
Christian.
> -Daniel
>
>> *bo_ptr = bo;
>> return 0;
>> }
>> --
>> 2.27.0
>>
^ permalink raw reply [flat|nested] 6+ messages in thread