diff mbox series

[04/10] drm/tegra: Set fbdev flags

Message ID 20230704160133.20261-5-tzimmermann@suse.de
State New
Headers show
Series drm: Improve fbdev emulation for DMA-able framebuffers | expand

Commit Message

Thomas Zimmermann July 4, 2023, 3:50 p.m. UTC
Set fbdev default flags FBNFO_DEFAULT and mark the framebuffer with
FBINFO_VIRTFB. The framebuffer range is in DMA-able memory and should
be accessed with the CPU's regular memory ops.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Mikko Perttunen <mperttunen@nvidia.com>
---
 drivers/gpu/drm/tegra/fbdev.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Javier Martinez Canillas July 5, 2023, 8:34 a.m. UTC | #1
Thomas Zimmermann <tzimmermann@suse.de> writes:

> Set fbdev default flags FBNFO_DEFAULT and mark the framebuffer with
> FBINFO_VIRTFB. The framebuffer range is in DMA-able memory and should
> be accessed with the CPU's regular memory ops.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Mikko Perttunen <mperttunen@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/fbdev.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/tegra/fbdev.c b/drivers/gpu/drm/tegra/fbdev.c
> index 82577b7c88da..8074430c52f1 100644
> --- a/drivers/gpu/drm/tegra/fbdev.c
> +++ b/drivers/gpu/drm/tegra/fbdev.c
> @@ -103,6 +103,8 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
>  		return PTR_ERR(info);
>  	}
>  
> +	info->flags = FBINFO_DEFAULT;
> +
>  	fb = tegra_fb_alloc(drm, &cmd, &bo, 1);
>  	if (IS_ERR(fb)) {
>  		err = PTR_ERR(fb);
> @@ -132,6 +134,7 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
>  		}
>  	}
>  
> +	info->flags |= FBINFO_VIRTFB;

I see that all fbdev drivers just do: info->flags = FBINFO_FLAG_DEFAULT | FBINFO_VIRTFB

Guess you are doing in two assignments to be consistent with drm_fbdev_dma.c ?
I was just curious about the rationale for setting the flags in two steps.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Thomas Zimmermann July 6, 2023, 12:44 p.m. UTC | #2
Hi

Am 05.07.23 um 11:34 schrieb Javier Martinez Canillas:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
> 
> [...]
>     
>>>> +	info->flags |= FBINFO_VIRTFB;
>>>
>>> I see that all fbdev drivers just do: info->flags = FBINFO_FLAG_DEFAULT | FBINFO_VIRTFB
>>>
>>> Guess you are doing in two assignments to be consistent with drm_fbdev_dma.c ?
>>> I was just curious about the rationale for setting the flags in two steps.
>>
>> The _DEFAULT flag is really just a zero. And the other flags describe
>> different aspects of the framebuffer.  I think it makes sense to set the
>> flags together with the respective state. For example, _VIRTFB is set
>> next to ->screen_buffer, because they belong together.
>>
> 
> Yes, that makes sense.
> 
>> _VIRTFB is currently only used in defio code at
>>
>> https://elixir.bootlin.com/linux/latest/source/drivers/video/fbdev/core/fb_defio.c#L232
>>
>> I think the fbdev I/O helpers should also test this flag after all
>> drivers have been annotated correctly. For example, fb_io_read() would
>> WARN_ONCE if the _VIRTFB flag has been set; and fb_sys_read() would warn
>> if it hasn't been set.  For the read helpers, it also makes sense to
>> WARN_ONCE if the _READS_FAST flag has not been set.
>>
> 
> Agreed. Maybe you could add those warn (or at least info or debug?) even
> if not all drivers have been annotated correctly. That way people can be
> aware that is missing and fix if there are remaining drivers.

Yes, we could do that. I want to go through drivers first and fix the 
low-hanging fruits. Some of the old fbdev drivers use either DMA or I/O 
memory. They would only be fix-worthy if someone complains.

Best regards
Thomas

> 
>> Best regards
>> Thomas
>>
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tegra/fbdev.c b/drivers/gpu/drm/tegra/fbdev.c
index 82577b7c88da..8074430c52f1 100644
--- a/drivers/gpu/drm/tegra/fbdev.c
+++ b/drivers/gpu/drm/tegra/fbdev.c
@@ -103,6 +103,8 @@  static int tegra_fbdev_probe(struct drm_fb_helper *helper,
 		return PTR_ERR(info);
 	}
 
+	info->flags = FBINFO_DEFAULT;
+
 	fb = tegra_fb_alloc(drm, &cmd, &bo, 1);
 	if (IS_ERR(fb)) {
 		err = PTR_ERR(fb);
@@ -132,6 +134,7 @@  static int tegra_fbdev_probe(struct drm_fb_helper *helper,
 		}
 	}
 
+	info->flags |= FBINFO_VIRTFB;
 	info->screen_base = (void __iomem *)bo->vaddr + offset;
 	info->screen_size = size;
 	info->fix.smem_start = (unsigned long)(bo->iova + offset);