Re: [PATCH v5 79/80] drm/omapdrm: Remove struct drm_fb_helper from struct omap_fbdev.

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

Hi,

On 27/09/2024 17:46, Thomas Zimmermann wrote:
> 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@suse.de>
>>> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>>> Acked-by: Javier Martinez Canillas <javierm@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@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.

Personally I'm fine with dropping it, but I have to say the console 
scrolling looked reaaaally slow without it. I'll try to find the time to 
at least figure out when it broke. Maybe it was a recent thing, and thus 
no one has complained.

  Tomi