[PATCH v5 79/80] drm/omapdrm: Remove struct drm_fb_helper from struct omap_fbdev.
Tomi Valkeinen
tomi.valkeinen at ideasonboard.com
Thu Sep 26 13:07:29 UTC 2024
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);
> }
More information about the amd-gfx
mailing list