| Message ID | 20220303205839.28484-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | drm: Support GEM SHMEM fbdev without shadow FB | expand |
On 3/3/22 21:58, Thomas Zimmermann wrote: > Don't select shadow buffering for the fbdev console explicitly. The > fbdev emulation's heuristic will enable it for any framebuffer with > .dirty callback. > Indeed it does. Not related to your series but looking at this patch I noticed that drivers/gpu/drm/tiny/bochs.c will be the only driver that sets .prefer_shadow_fbdev after this lands. The driver is using GEM so I wonder if after your series this DRM driver could have a .dirty callback and the field just be dropped? Or there would still be a case where it is needed ? Anyway, just wanted to mention in case I forget later. Your patch looks good to me and I think it could be pushed without waiting for the other patches in the series. Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Am 08.03.22 um 10:31 schrieb Javier Martinez Canillas: > On 3/3/22 21:58, Thomas Zimmermann wrote: >> Don't select shadow buffering for the fbdev console explicitly. The >> fbdev emulation's heuristic will enable it for any framebuffer with >> .dirty callback. >> > > Indeed it does. Not related to your series but looking at this > patch I noticed that drivers/gpu/drm/tiny/bochs.c will be the > only driver that sets .prefer_shadow_fbdev after this lands. > > The driver is using GEM so I wonder if after your series this > DRM driver could have a .dirty callback and the field just be > dropped? Or there would still be a case where it is needed ? Bochs uses VRAM helpers (i.e., TTM). Fbdev and userspace would directly write into that buffer memory without a copy. So the dirty function would be empty. Other drivers with VRAM helpers (e.g., hibmc, ast) operate on uncached I/O memory AFAICT. So they set .prefer_shadow, which also affects userspace. Bochs uses cached memory and shouldn't need prefer_shadow. Setting prefer_shadow_fbdev is only there for making the fbdev buffer object evictable from video memory. As it stands, using prefer_shadow_fbdev is the cleanest solution, even if bochs is the only user of that field. Alternatively, we could make it a requirement that qemu provides enough video memory for bochs to unconditionally pin the fbdev BO there without ever evicting. I guess, that would mean 32 MiB of VRAM at least. Best regards Thomas > > Anyway, just wanted to mention in case I forget later. > > Your patch looks good to me and I think it could be pushed > without waiting for the other patches in the series. > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >
On 3/8/22 10:56, Thomas Zimmermann wrote: > Hi > > Am 08.03.22 um 10:31 schrieb Javier Martinez Canillas: >> On 3/3/22 21:58, Thomas Zimmermann wrote: >>> Don't select shadow buffering for the fbdev console explicitly. The >>> fbdev emulation's heuristic will enable it for any framebuffer with >>> .dirty callback. >>> >> >> Indeed it does. Not related to your series but looking at this >> patch I noticed that drivers/gpu/drm/tiny/bochs.c will be the >> only driver that sets .prefer_shadow_fbdev after this lands. >> >> The driver is using GEM so I wonder if after your series this >> DRM driver could have a .dirty callback and the field just be >> dropped? Or there would still be a case where it is needed ? > Bochs uses VRAM helpers (i.e., TTM). Fbdev and userspace would directly > write into that buffer memory without a copy. So the dirty function > would be empty. > > Other drivers with VRAM helpers (e.g., hibmc, ast) operate on uncached > I/O memory AFAICT. So they set .prefer_shadow, which also affects > userspace. Bochs uses cached memory and shouldn't need prefer_shadow. > Setting prefer_shadow_fbdev is only there for making the fbdev buffer > object evictable from video memory. > > As it stands, using prefer_shadow_fbdev is the cleanest solution, even > if bochs is the only user of that field. > > Alternatively, we could make it a requirement that qemu provides enough > video memory for bochs to unconditionally pin the fbdev BO there without > ever evicting. I guess, that would mean 32 MiB of VRAM at least. > I see. Thanks a lot for this explanation. > Best regards > Thomas >
diff --git a/drivers/gpu/drm/tiny/simpledrm.c b/drivers/gpu/drm/tiny/simpledrm.c index f5b8e864a5cd..768242a78e2b 100644 --- a/drivers/gpu/drm/tiny/simpledrm.c +++ b/drivers/gpu/drm/tiny/simpledrm.c @@ -801,7 +801,6 @@ static int simpledrm_device_init_modeset(struct simpledrm_device *sdev) dev->mode_config.max_width = max_width; dev->mode_config.min_height = mode->vdisplay; dev->mode_config.max_height = max_height; - dev->mode_config.prefer_shadow_fbdev = true; dev->mode_config.preferred_depth = sdev->format->cpp[0] * 8; dev->mode_config.funcs = &simpledrm_mode_config_funcs;
Don't select shadow buffering for the fbdev console explicitly. The fbdev emulation's heuristic will enable it for any framebuffer with .dirty callback. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/tiny/simpledrm.c | 1 - 1 file changed, 1 deletion(-)