Thomas Zimmermann
2023-Mar-20 12:44 UTC
[PATCH 4/6] drm/fbdev-generic: Clean up after failed probing
Hi Javier Am 17.03.23 um 13:24 schrieb Javier Martinez Canillas:> Thomas Zimmermann <tzimmermann at suse.de> writes: > > [...] > >> @@ -91,36 +93,52 @@ static int drm_fbdev_fb_probe(struct drm_fb_helper *fb_helper, >> >> fb_helper->buffer = buffer; >> fb_helper->fb = buffer->fb; >> - fb = buffer->fb; >> + >> + screen_size = buffer->gem->size; > > [...] > >> - info->screen_size = sizes->surface_height * fb->pitches[0]; > > [...] > > I wonder if this change should be a separate patch? I know that it should > be the same size but still feels like an unrelated change that deserves a > different patch and description.This comment made me look up the exact meaning if screen_size, which is "Amount of ioremapped VRAM or 0". [1] Other drivers with shadow buffers (udlfb, metronomefb) apparently don't set this field. So the generic fbdev probably shouldn't either. The size of the video memory (physical or shadowed) in in fix.smem_len. [2] From grep'ing through the source code, it's not clear to me why screen_size exists in the first place. Best regards Thomas [1] https://elixir.bootlin.com/linux/latest/source/include/linux/fb.h#L494 [2] https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/fb.h#L161> > [...] > >> - /* Set a default deferred I/O handler */ >> + /* deferred I/O */ > > I would either have it as /* Generic fbdev deferred I/O handler */ or just > remove the comment. I understand why you are removing the "default", since > implies that drivers can change the handler and that's not the case here. > > But I think that just leaving the "deferred I/O" comment there doesn't say > that much. > > Reviewed-by: Javier Martinez Canillas <javierm at redhat.com> >-- Thomas Zimmermann Graphics Driver Developer SUSE Software Solutions Germany GmbH Maxfeldstr. 5, 90409 N?rnberg, Germany (HRB 36809, AG N?rnberg) Gesch?ftsf?hrer: Ivo Totev -------------- next part -------------- A non-text attachment was scrubbed... Name: OpenPGP_signature Type: application/pgp-signature Size: 840 bytes Desc: OpenPGP digital signature URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20230320/383381c9/attachment.sig>