diff mbox series

[v3,1/5] gem/vram: pin to vram in vmap

Message ID 20190627122348.5833-2-kraxel@redhat.com (mailing list archive)
State New, archived
Headers show
Series drm/bochs: cleanups and framebuffer setup fix. | expand

Commit Message

Gerd Hoffmann June 27, 2019, 12:23 p.m. UTC
drm clients like the generic framebuffer emulation keep a permanent
vmap active, which in turn has a permanent pin.  This pin needs to
be in vram, otherwise we can't display the framebuffer.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Zimmermann June 27, 2019, 2:37 p.m. UTC | #1
Hi

Am 27.06.19 um 14:23 schrieb Gerd Hoffmann:
> drm clients like the generic framebuffer emulation keep a permanent
> vmap active, which in turn has a permanent pin.  This pin needs to
> be in vram, otherwise we can't display the framebuffer.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 4de782ca26b2..c724876c6f2a 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -592,7 +592,7 @@ void *drm_gem_vram_driver_gem_prime_vmap(struct drm_gem_object *gem)
>  	int ret;
>  	void *base;
>  
> -	ret = drm_gem_vram_pin(gbo, 0);
> +	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);

I have a patch set that converts ast and mgag200 to generic framebuffer
emulation with a shadow FB. The actual BO is mapped by the fbdev code on
demand to update its content. Permanently mapping the fb console's BO
would consume too much display memory. This requires the pin function's
placement flag to be 0, so the BO is mapped in system memory by default.
The proposed patch breaks this.

Some ideas for solving this:

 1) Introduce a default_placement field in struct drm_gem_vram_helper
where this flag can be configured. I'd favor this option.

 2) Introduce a separate callback function for pinning to vram. The
driver would have to set the correct function pointers.

 3) Pin the fb console buffer manually from within the bochs driver.

Best regards
Thomas

>  	if (ret)
>  		return NULL;
>  	base = drm_gem_vram_kmap(gbo, true, NULL);
>
Gerd Hoffmann June 27, 2019, 3:16 p.m. UTC | #2
Hi,

>  1) Introduce a default_placement field in struct drm_gem_vram_helper
> where this flag can be configured. I'd favor this option.

>  2) Introduce a separate callback function for pinning to vram. The
> driver would have to set the correct function pointers.

>  3) Pin the fb console buffer manually from within the bochs driver.

Hmm.  Before calling drm_fbdev_generic_setup() the bo doesn't exist yet
and when the function returns it is already vmapped and pinned I think.

So (3) isn't easily doable.  (1) looks best to me.

cheers,
  Gerd
Thomas Zimmermann June 27, 2019, 3:54 p.m. UTC | #3
Hi

Am 27.06.19 um 17:16 schrieb Gerd Hoffmann:
>   Hi,
> 
>>  1) Introduce a default_placement field in struct drm_gem_vram_helper
>> where this flag can be configured. I'd favor this option.
> 
>>  2) Introduce a separate callback function for pinning to vram. The
>> driver would have to set the correct function pointers.
> 
>>  3) Pin the fb console buffer manually from within the bochs driver.
> 
> Hmm.  Before calling drm_fbdev_generic_setup() the bo doesn't exist yet
> and when the function returns it is already vmapped and pinned I think.
> 
> So (3) isn't easily doable.  (1) looks best to me.

For my patches, it's OK to have to BO pinned to VRAM by default. As the
BO will be unmapped most of the time, I can change this flag at any time.

Best regards
Thomas

> cheers,
>   Gerd
>
Thomas Zimmermann June 27, 2019, 3:59 p.m. UTC | #4
Am 27.06.19 um 17:54 schrieb Thomas Zimmermann:
> Hi
> 
> Am 27.06.19 um 17:16 schrieb Gerd Hoffmann:
>>   Hi,
>>
>>>  1) Introduce a default_placement field in struct drm_gem_vram_helper
>>> where this flag can be configured. I'd favor this option.
>>
>>>  2) Introduce a separate callback function for pinning to vram. The
>>> driver would have to set the correct function pointers.
>>
>>>  3) Pin the fb console buffer manually from within the bochs driver.
>>
>> Hmm.  Before calling drm_fbdev_generic_setup() the bo doesn't exist yet
>> and when the function returns it is already vmapped and pinned I think.
>>
>> So (3) isn't easily doable.  (1) looks best to me.
> 
> For my patches, it's OK to have to BO pinned to VRAM by default. As the
> BO will be unmapped most of the time, I can change this flag at any time.

So, yeah, option 1 ...

> Best regards
> Thomas
> 
>> cheers,
>>   Gerd
>>
> 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 4de782ca26b2..c724876c6f2a 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -592,7 +592,7 @@  void *drm_gem_vram_driver_gem_prime_vmap(struct drm_gem_object *gem)
 	int ret;
 	void *base;
 
-	ret = drm_gem_vram_pin(gbo, 0);
+	ret = drm_gem_vram_pin(gbo, DRM_GEM_VRAM_PL_FLAG_VRAM);
 	if (ret)
 		return NULL;
 	base = drm_gem_vram_kmap(gbo, true, NULL);