Hi Javier Am 17.03.23 um 13:24 schrieb Javier Martinez Canillas:
Thomas Zimmermann <tzimmermann@xxxxxxx> 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@xxxxxxxxxx>
-- 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
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
