[PATCH v5 79/80] drm/omapdrm: Remove struct drm_fb_helper from struct omap_fbdev.
Thomas Zimmermann
tzimmermann at suse.de
Fri Sep 27 14:46:07 UTC 2024
Hi
Am 26.09.24 um 15:07 schrieb Tomi Valkeinen:
> 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?
If ywrap doesn't work and no one complained, can we remove it? The rest
of the code is equivalent to drm_fbdev_dma.c, so we could remove omap's
implementation entirely.
Best regards
Thomas
>
> 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);
>> }
>
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
More information about the dri-devel
mailing list