Thomas Zimmermann
2024-Sep-24 07:13 UTC
[PATCH v5 79/80] drm/omapdrm: Remove struct drm_fb_helper from struct omap_fbdev.
Store instances of drm_fb_helper and struct omap_fbdev separately.
This will allow omapdrm to use the common fbdev client, which allocates
its own instance of struct drm_fb_helper.
There is at most one instance of each per DRM device, so both can be
referenced directly from the omap and DRM device structures. A later
patchset might rework the common fbdev client to allow for storing
both, drm_fb_helper and omap_fbdev, together in the same place.
Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
Cc: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com>
Acked-by: Javier Martinez Canillas <javierm at redhat.com>
---
drivers/gpu/drm/omapdrm/omap_drv.h | 3 ++
drivers/gpu/drm/omapdrm/omap_fbdev.c | 42 +++++++++++++++-------------
2 files changed, 26 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
b/drivers/gpu/drm/omapdrm/omap_drv.h
index 4c7217b35f6b..d903568fd8cc 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -32,6 +32,7 @@
#define MODULE_NAME "omapdrm"
struct omap_drm_usergart;
+struct omap_fbdev;
struct omap_drm_pipeline {
struct drm_crtc *crtc;
@@ -97,6 +98,8 @@ struct omap_drm_private {
/* memory bandwidth limit if it is needed on the platform */
unsigned int max_bandwidth;
+
+ struct omap_fbdev *fbdev;
};
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c
b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 523be34682ca..044e80403c3b 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -13,6 +13,7 @@
#include <drm/drm_fourcc.h>
#include <drm/drm_framebuffer.h>
#include <drm/drm_gem_framebuffer_helper.h>
+#include <drm/drm_managed.h>
#include <drm/drm_util.h>
#include "omap_drv.h"
@@ -26,10 +27,8 @@ module_param_named(ywrap, ywrap_enabled, bool, 0644);
* fbdev funcs, to implement legacy fbdev interface on top of drm driver
*/
-#define to_omap_fbdev(x) container_of(x, struct omap_fbdev, base)
-
struct omap_fbdev {
- struct drm_fb_helper base;
+ struct drm_device *dev;
bool ywrap_enabled;
/* for deferred dmm roll when getting called in atomic ctx */
@@ -41,7 +40,7 @@ static struct drm_fb_helper *get_fb(struct fb_info *fbi);
static void pan_worker(struct work_struct *work)
{
struct omap_fbdev *fbdev = container_of(work, struct omap_fbdev, work);
- struct drm_fb_helper *helper = &fbdev->base;
+ struct drm_fb_helper *helper = fbdev->dev->fb_helper;
struct fb_info *fbi = helper->info;
struct drm_gem_object *bo = drm_gem_fb_get_obj(helper->fb, 0);
int npages;
@@ -55,24 +54,25 @@ FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(omap_fbdev,
drm_fb_helper_damage_range,
drm_fb_helper_damage_area)
-static int omap_fbdev_pan_display(struct fb_var_screeninfo *var,
- struct fb_info *fbi)
+static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, struct fb_info
*fbi)
{
struct drm_fb_helper *helper = get_fb(fbi);
- struct omap_fbdev *fbdev = to_omap_fbdev(helper);
+ struct omap_drm_private *priv;
+ struct omap_fbdev *fbdev;
if (!helper)
goto fallback;
+ priv = helper->dev->dev_private;
+ fbdev = priv->fbdev;
+
if (!fbdev->ywrap_enabled)
goto fallback;
- if (drm_can_sleep()) {
+ if (drm_can_sleep())
pan_worker(&fbdev->work);
- } else {
- struct omap_drm_private *priv = helper->dev->dev_private;
+ else
queue_work(priv->wq, &fbdev->work);
- }
return 0;
@@ -92,7 +92,6 @@ static void omap_fbdev_fb_destroy(struct fb_info *info)
struct drm_fb_helper *helper = info->par;
struct drm_framebuffer *fb = helper->fb;
struct drm_gem_object *bo = drm_gem_fb_get_obj(fb, 0);
- struct omap_fbdev *fbdev = to_omap_fbdev(helper);
DBG();
@@ -104,7 +103,7 @@ static void omap_fbdev_fb_destroy(struct fb_info *info)
drm_client_release(&helper->client);
drm_fb_helper_unprepare(helper);
- kfree(fbdev);
+ kfree(helper);
}
/*
@@ -128,9 +127,9 @@ static const struct fb_ops omap_fb_ops = {
static int omap_fbdev_create(struct drm_fb_helper *helper,
struct drm_fb_helper_surface_size *sizes)
{
- struct omap_fbdev *fbdev = to_omap_fbdev(helper);
struct drm_device *dev = helper->dev;
struct omap_drm_private *priv = dev->dev_private;
+ struct omap_fbdev *fbdev = priv->fbdev;
struct drm_framebuffer *fb = NULL;
union omap_gem_size gsize;
struct fb_info *fbi = NULL;
@@ -338,6 +337,7 @@ static const struct drm_client_funcs omap_fbdev_client_funcs
= {
void omap_fbdev_setup(struct drm_device *dev)
{
+ struct omap_drm_private *priv = dev->dev_private;
struct omap_fbdev *fbdev;
struct drm_fb_helper *helper;
int ret;
@@ -345,24 +345,28 @@ void omap_fbdev_setup(struct drm_device *dev)
drm_WARN(dev, !dev->registered, "Device has not been
registered.\n");
drm_WARN(dev, dev->fb_helper, "fb_helper is already set!\n");
- fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL);
+ fbdev = drmm_kzalloc(dev, sizeof(*fbdev), GFP_KERNEL);
if (!fbdev)
return;
- helper = &fbdev->base;
+ fbdev->dev = dev;
+ INIT_WORK(&fbdev->work, pan_worker);
+
+ priv->fbdev = fbdev;
+ helper = kzalloc(sizeof(*helper), GFP_KERNEL);
+ if (!helper)
+ return;
drm_fb_helper_prepare(dev, helper, 32, &omap_fb_helper_funcs);
ret = drm_client_init(dev, &helper->client, "fbdev",
&omap_fbdev_client_funcs);
if (ret)
goto err_drm_client_init;
- INIT_WORK(&fbdev->work, pan_worker);
-
drm_client_register(&helper->client);
return;
err_drm_client_init:
drm_fb_helper_unprepare(helper);
- kfree(fbdev);
+ kfree(helper);
}
--
2.46.0
Tomi Valkeinen
2024-Sep-26 13:07 UTC
[PATCH v5 79/80] drm/omapdrm: Remove struct drm_fb_helper from struct omap_fbdev.
Hi, On 24/09/2024 10:13, Thomas Zimmermann wrote:> Store instances of drm_fb_helper and struct omap_fbdev separately. > This will allow omapdrm to use the common fbdev client, which allocates > its own instance of struct drm_fb_helper. > > There is at most one instance of each per DRM device, so both can be > referenced directly from the omap and DRM device structures. A later > patchset might rework the common fbdev client to allow for storing > both, drm_fb_helper and omap_fbdev, together in the same place. > > Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de> > Cc: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com> > Acked-by: Javier Martinez Canillas <javierm at redhat.com> > --- > drivers/gpu/drm/omapdrm/omap_drv.h | 3 ++ > drivers/gpu/drm/omapdrm/omap_fbdev.c | 42 +++++++++++++++------------- > 2 files changed, 26 insertions(+), 19 deletions(-)The ywrap seems to be broken, but it's broken without this series too. With some quick hacks I managed to get the omapdrm's panning function called, and it works the same way with or without this series. Tested on DRA76 EVM. Reviewed-by: Tomi Valkeinen <tomi.valkeinen at ideasonboard.com> For reference, in case someone is interested fixing omapdrm's ywrap code: - get_fb() doesn't work. I don't understand the point of the function. It compares fbi->fix.id and MODULE_NAME, and the "fbi->fix.id" has changed along the years so the check no longer works. Afaik, the whole function is not needed, and we can just use fbi->par. - omap_fbdev_pan_display() doesn't seem to get called when the console scrolls. I can get it called with an userspace app that does some y panning. Maybe FBINFO_HWACCEL_YWRAP is not supported anymore? Tomi> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h > index 4c7217b35f6b..d903568fd8cc 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.h > +++ b/drivers/gpu/drm/omapdrm/omap_drv.h > @@ -32,6 +32,7 @@ > #define MODULE_NAME "omapdrm" > > struct omap_drm_usergart; > +struct omap_fbdev; > > struct omap_drm_pipeline { > struct drm_crtc *crtc; > @@ -97,6 +98,8 @@ struct omap_drm_private { > > /* memory bandwidth limit if it is needed on the platform */ > unsigned int max_bandwidth; > + > + struct omap_fbdev *fbdev; > }; > > > diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c > index 523be34682ca..044e80403c3b 100644 > --- a/drivers/gpu/drm/omapdrm/omap_fbdev.c > +++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c > @@ -13,6 +13,7 @@ > #include <drm/drm_fourcc.h> > #include <drm/drm_framebuffer.h> > #include <drm/drm_gem_framebuffer_helper.h> > +#include <drm/drm_managed.h> > #include <drm/drm_util.h> > > #include "omap_drv.h" > @@ -26,10 +27,8 @@ module_param_named(ywrap, ywrap_enabled, bool, 0644); > * fbdev funcs, to implement legacy fbdev interface on top of drm driver > */ > > -#define to_omap_fbdev(x) container_of(x, struct omap_fbdev, base) > - > struct omap_fbdev { > - struct drm_fb_helper base; > + struct drm_device *dev; > bool ywrap_enabled; > > /* for deferred dmm roll when getting called in atomic ctx */ > @@ -41,7 +40,7 @@ static struct drm_fb_helper *get_fb(struct fb_info *fbi); > static void pan_worker(struct work_struct *work) > { > struct omap_fbdev *fbdev = container_of(work, struct omap_fbdev, work); > - struct drm_fb_helper *helper = &fbdev->base; > + struct drm_fb_helper *helper = fbdev->dev->fb_helper; > struct fb_info *fbi = helper->info; > struct drm_gem_object *bo = drm_gem_fb_get_obj(helper->fb, 0); > int npages; > @@ -55,24 +54,25 @@ FB_GEN_DEFAULT_DEFERRED_DMAMEM_OPS(omap_fbdev, > drm_fb_helper_damage_range, > drm_fb_helper_damage_area) > > -static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, > - struct fb_info *fbi) > +static int omap_fbdev_pan_display(struct fb_var_screeninfo *var, struct fb_info *fbi) > { > struct drm_fb_helper *helper = get_fb(fbi); > - struct omap_fbdev *fbdev = to_omap_fbdev(helper); > + struct omap_drm_private *priv; > + struct omap_fbdev *fbdev; > > if (!helper) > goto fallback; > > + priv = helper->dev->dev_private; > + fbdev = priv->fbdev; > + > if (!fbdev->ywrap_enabled) > goto fallback; > > - if (drm_can_sleep()) { > + if (drm_can_sleep()) > pan_worker(&fbdev->work); > - } else { > - struct omap_drm_private *priv = helper->dev->dev_private; > + else > queue_work(priv->wq, &fbdev->work); > - } > > return 0; > > @@ -92,7 +92,6 @@ static void omap_fbdev_fb_destroy(struct fb_info *info) > struct drm_fb_helper *helper = info->par; > struct drm_framebuffer *fb = helper->fb; > struct drm_gem_object *bo = drm_gem_fb_get_obj(fb, 0); > - struct omap_fbdev *fbdev = to_omap_fbdev(helper); > > DBG(); > > @@ -104,7 +103,7 @@ static void omap_fbdev_fb_destroy(struct fb_info *info) > > drm_client_release(&helper->client); > drm_fb_helper_unprepare(helper); > - kfree(fbdev); > + kfree(helper); > } > > /* > @@ -128,9 +127,9 @@ static const struct fb_ops omap_fb_ops = { > static int omap_fbdev_create(struct drm_fb_helper *helper, > struct drm_fb_helper_surface_size *sizes) > { > - struct omap_fbdev *fbdev = to_omap_fbdev(helper); > struct drm_device *dev = helper->dev; > struct omap_drm_private *priv = dev->dev_private; > + struct omap_fbdev *fbdev = priv->fbdev; > struct drm_framebuffer *fb = NULL; > union omap_gem_size gsize; > struct fb_info *fbi = NULL; > @@ -338,6 +337,7 @@ static const struct drm_client_funcs omap_fbdev_client_funcs = { > > void omap_fbdev_setup(struct drm_device *dev) > { > + struct omap_drm_private *priv = dev->dev_private; > struct omap_fbdev *fbdev; > struct drm_fb_helper *helper; > int ret; > @@ -345,24 +345,28 @@ void omap_fbdev_setup(struct drm_device *dev) > drm_WARN(dev, !dev->registered, "Device has not been registered.\n"); > drm_WARN(dev, dev->fb_helper, "fb_helper is already set!\n"); > > - fbdev = kzalloc(sizeof(*fbdev), GFP_KERNEL); > + fbdev = drmm_kzalloc(dev, sizeof(*fbdev), GFP_KERNEL); > if (!fbdev) > return; > - helper = &fbdev->base; > + fbdev->dev = dev; > + INIT_WORK(&fbdev->work, pan_worker); > + > + priv->fbdev = fbdev; > > + helper = kzalloc(sizeof(*helper), GFP_KERNEL); > + if (!helper) > + return; > drm_fb_helper_prepare(dev, helper, 32, &omap_fb_helper_funcs); > > ret = drm_client_init(dev, &helper->client, "fbdev", &omap_fbdev_client_funcs); > if (ret) > goto err_drm_client_init; > > - INIT_WORK(&fbdev->work, pan_worker); > - > drm_client_register(&helper->client); > > return; > > err_drm_client_init: > drm_fb_helper_unprepare(helper); > - kfree(fbdev); > + kfree(helper); > }
Apparently Analagous Threads
- [PATCH v2 78/86] drm/radeon: Run DRM default client setup
- [Bug 23730] New: x11-base/nouveau-drm fails on drm_fb_helper.c with 'EINVAL' undeclared
- [PATCH 07/16] drm/omapdrm: removed optional dummy crtc mode_fixup function.
- [PATCH 10/20] drm/omapdrm: Introduce GEM object functions
- [PATCH v3 00/28] DRM API Conversions