| Message ID | 20230704160133.20261-7-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm: Improve fbdev emulation for DMA-able framebuffers | expand |
Thomas Zimmermann <tzimmermann@suse.de> writes: > Set fbdev default flags FBNFO_DEFAULT and mark the framebuffer with FBINFO_DEFAULT, or did you meand FBINFO_FLAG_DEFAULT (the flag your patch is actually using) ? I just noticed that are the same... and in patch 04/10 you used the former for the tegra driver, but here you are using the latter. Is on purpose or just a mistake ? > 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: Inki Dae <inki.dae@samsung.com> > Cc: Seung-Woo Kim <sw0312.kim@samsung.com> > Cc: Kyungmin Park <kyungmin.park@samsung.com> > Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Cc: Alim Akhtar <alim.akhtar@samsung.com> > --- > drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > index 7ca3424b59ce..28dc398d6e10 100644 > --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c > @@ -72,6 +72,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper, > return PTR_ERR(fbi); > } > > + fbi->flags = FBINFO_FLAG_DEFAULT; The #define FBINFO_FLAG_DEFAULT FBINFO_DEFAULT seems to be there since the original v2.6.12-rc2 git import in commit 1da177e4c3f4, so is hard to know why was introduced. FBINFO_DEFAULT is more used, I will just stick to that: $ git grep FBINFO_DEFAULT | wc -l 92 $ git grep FBINFO_FLAG_DEFAULT | wc -l 38 Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Hi Am 05.07.23 um 10:49 schrieb Javier Martinez Canillas: > Thomas Zimmermann <tzimmermann@suse.de> writes: > >> Set fbdev default flags FBNFO_DEFAULT and mark the framebuffer with > > FBINFO_DEFAULT, or did you meand FBINFO_FLAG_DEFAULT (the flag your patch > is actually using) ? > > I just noticed that are the same... and in patch 04/10 you used the former > for the tegra driver, but here you are using the latter. Is on purpose or > just a mistake ? > >> 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: Inki Dae <inki.dae@samsung.com> >> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> >> Cc: Kyungmin Park <kyungmin.park@samsung.com> >> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> >> Cc: Alim Akhtar <alim.akhtar@samsung.com> >> --- >> drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> index 7ca3424b59ce..28dc398d6e10 100644 >> --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c >> @@ -72,6 +72,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper, >> return PTR_ERR(fbi); >> } >> >> + fbi->flags = FBINFO_FLAG_DEFAULT; > > The #define FBINFO_FLAG_DEFAULT FBINFO_DEFAULT seems to be there since the > original v2.6.12-rc2 git import in commit 1da177e4c3f4, so is hard to know > why was introduced. FBINFO_DEFAULT is more used, I will just stick to that: Thanks for commenting on this issue. I didn't notice that. I think I'll just remove these _DEFAULT assignments from the patchset. And I think we should nuke them entirely everywhere. The _DEFAULT values are just 0 after commit 376b3ff54c9a1. So there's no value in assigning them at all. Best regards Thomas > > $ git grep FBINFO_DEFAULT | wc -l > 92 > > $ git grep FBINFO_FLAG_DEFAULT | wc -l > 38 > > Reviewed-by: Javier Martinez Canillas <javierm@redhat.com> >
Thomas Zimmermann <tzimmermann@suse.de> writes: > Hi > > Am 05.07.23 um 10:49 schrieb Javier Martinez Canillas: [...] >> >> The #define FBINFO_FLAG_DEFAULT FBINFO_DEFAULT seems to be there since the >> original v2.6.12-rc2 git import in commit 1da177e4c3f4, so is hard to know >> why was introduced. FBINFO_DEFAULT is more used, I will just stick to that: > > Thanks for commenting on this issue. I didn't notice that. > > I think I'll just remove these _DEFAULT assignments from the patchset. > > And I think we should nuke them entirely everywhere. The _DEFAULT values > are just 0 after commit 376b3ff54c9a1. So there's no value in assigning > them at all. > Agreed.
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index 7ca3424b59ce..28dc398d6e10 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -72,6 +72,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper, return PTR_ERR(fbi); } + fbi->flags = FBINFO_FLAG_DEFAULT; fbi->fbops = &exynos_drm_fb_ops; drm_fb_helper_fill_info(fbi, helper, sizes); @@ -79,6 +80,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper, offset = fbi->var.xoffset * fb->format->cpp[0]; offset += fbi->var.yoffset * fb->pitches[0]; + fbi->flags |= FBINFO_VIRTFB; fbi->screen_buffer = exynos_gem->kvaddr + offset; fbi->screen_size = size; fbi->fix.smem_len = size;
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: Inki Dae <inki.dae@samsung.com> Cc: Seung-Woo Kim <sw0312.kim@samsung.com> Cc: Kyungmin Park <kyungmin.park@samsung.com> Cc: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Cc: Alim Akhtar <alim.akhtar@samsung.com> --- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 ++ 1 file changed, 2 insertions(+)