[Intel-gfx] [PATCH] drm/i915/fbdev: Implement wrappers for callbacks used by fbcon
Thomas Zimmermann
tzimmermann at suse.de
Fri Feb 3 11:42:01 UTC 2023
Hi
Am 03.02.23 um 12:09 schrieb Hogander, Jouni:
> On Fri, 2023-02-03 at 12:42 +0200, Ville Syrjälä wrote:
>> On Fri, Feb 03, 2023 at 07:21:27AM +0000, Hogander, Jouni wrote:
>>> On Tue, 2023-01-24 at 13:27 +0100, Thomas Zimmermann wrote:
>>>> Hi
>>>>
>>>> Am 24.01.23 um 10:10 schrieb Jouni Högander:
>>>>> After disconnecting damage worker from update logic our dirty
>>>>> callback
>>>>> is not called on fbcon events. This is causing problems to
>>>>> features
>>>>> (PSR, FBC, DRRS) relying on dirty callback getting called and
>>>>> breaking
>>>>> fb console when these features are in use.
>>>>>
>>>>> Implement wrappers for callbacks used by fbcon and call our
>>>>> dirty
>>>>> callback in those.
>>>>>
>>>>> Fixes: f231af498c29 ("drm/fb-helper: Disconnect damage worker
>>>>> from
>>>>> update logic")
>>>>> Cc: Ville Syrjälä <ville.syrjala at linux.intel.com>
>>>>> Cc: Thomas Zimmermann <tzimmermann at suse.de>
>>>>> Cc: Jani Nikula <jani.nikula at intel.com>
>>>>> Signed-off-by: Jouni Högander <jouni.hogander at intel.com>
>>>>
>>>> This is the better solution wrt what fbdev wants.
>>>
>>> There was a failure from testing robot. drivers/tty/vt/vt.c is
>>> using
>>> spinlock and in our dirty callback we are taking mutex.
I think I've seen this problem before in the context of vc4. We'd
probably have to add a damage worker to i915, which seems a bit much for
this fix.
>>>
>>> Do you have any suggestions? Shall we fallback to original fix
>>> which
>>> was setting the dirty callback where call is made from workqueue?
>>
>> Please just fix the original regression as straightforwardly as
>> possible.
>
> Here is the original patch by me:
>
> https://patchwork.freedesktop.org/series/111433/
Given the circumstances,
Reviewed-by: Thomas Zimmermann <tzimmermann at suse.de>
for this older patch. Maybe add a TODO comment that says that the
fb_dirty worker needs to be moved into i915 and that the fb_ write ops
need to be reimplemented for a good solution.
Best regards
Thomas
>
> which was eventually nacked by Thomas. Maybe we could continue with
> that one and solve these problems when/if fb_dirty is removed?
>
>>
>>>
>>>>
>>>> Acked-by: Thomas Zimmermann <tzimmermann at suse.de>
>>>>
>>>> Best regards
>>>> Thomas
>>>>
>>>>> ---
>>>>> drivers/gpu/drm/i915/display/intel_fbdev.c | 53
>>>>> ++++++++++++++++++++--
>>>>> 1 file changed, 49 insertions(+), 4 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> index 19f3b5d92a55..b1653624552e 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
>>>>> @@ -77,6 +77,18 @@ static void intel_fbdev_invalidate(struct
>>>>> intel_fbdev *ifbdev)
>>>>> intel_frontbuffer_invalidate(to_frontbuffer(ifbdev),
>>>>> ORIGIN_CPU);
>>>>> }
>>>>>
>>>>> +static void intel_fbdev_dirty(struct fb_info *info)
>>>>> +{
>>>>> + struct drm_fb_helper *helper = info->par;
>>>>> +
>>>>> + /*
>>>>> + * Intel_fb dirty implementation doesn't use damage
>>>>> clips -
>>>>>>
>>>>> + * no need to pass them here
>>>>> + */
>>>>> + if (helper->fb->funcs->dirty)
>>>>> + helper->fb->funcs->dirty(helper->fb, NULL, 0,
>>>>> 0,
>>>>> NULL, 0);
>>>>> +}
>>>>> +
>>>>> static int intel_fbdev_set_par(struct fb_info *info)
>>>>> {
>>>>> struct drm_fb_helper *fb_helper = info->par;
>>>>> @@ -91,6 +103,39 @@ static int intel_fbdev_set_par(struct
>>>>> fb_info
>>>>> *info)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> +static ssize_t intel_fbdev_write(struct fb_info *info, const
>>>>> char
>>>>> __user *buf,
>>>>> + size_t count, loff_t *ppos)
>>>>> +{
>>>>> + int ret;
>>>>> +
>>>>> + ret = drm_fb_helper_cfb_write(info, buf, count, ppos);
>>>>> + if (ret > 0)
>>>>> + intel_fbdev_dirty(info);
>>>>> +
>>>>> + return ret;
>>>>> +}
>>>>> +
>>>>> +static void intel_fbdev_fillrect(struct fb_info *info,
>>>>> + const struct fb_fillrect
>>>>> *rect)
>>>>> +{
>>>>> + drm_fb_helper_cfb_fillrect(info, rect);
>>>>> + intel_fbdev_dirty(info);
>>>>> +}
>>>>> +
>>>>> +static void intel_fbdev_copyarea(struct fb_info *info,
>>>>> + const struct fb_copyarea
>>>>> *area)
>>>>> +{
>>>>> + drm_fb_helper_cfb_copyarea(info, area);
>>>>> + intel_fbdev_dirty(info);
>>>>> +}
>>>>> +
>>>>> +static void intel_fbdev_imageblit(struct fb_info *info,
>>>>> + const struct fb_image *image)
>>>>> +{
>>>>> + drm_fb_helper_cfb_imageblit(info, image);
>>>>> + intel_fbdev_dirty(info);
>>>>> +}
>>>>> +
>>>>> static int intel_fbdev_blank(int blank, struct fb_info *info)
>>>>> {
>>>>> struct drm_fb_helper *fb_helper = info->par;
>>>>> @@ -125,10 +170,10 @@ static const struct fb_ops intelfb_ops =
>>>>> {
>>>>> DRM_FB_HELPER_DEFAULT_OPS,
>>>>> .fb_set_par = intel_fbdev_set_par,
>>>>> .fb_read = drm_fb_helper_cfb_read,
>>>>> - .fb_write = drm_fb_helper_cfb_write,
>>>>> - .fb_fillrect = drm_fb_helper_cfb_fillrect,
>>>>> - .fb_copyarea = drm_fb_helper_cfb_copyarea,
>>>>> - .fb_imageblit = drm_fb_helper_cfb_imageblit,
>>>>> + .fb_write = intel_fbdev_write,
>>>>> + .fb_fillrect = intel_fbdev_fillrect,
>>>>> + .fb_copyarea = intel_fbdev_copyarea,
>>>>> + .fb_imageblit = intel_fbdev_imageblit,
>>>>> .fb_pan_display = intel_fbdev_pan_display,
>>>>> .fb_blank = intel_fbdev_blank,
>>>>> };
>>>>
>>>> --
>>>> Thomas Zimmermann
>>>> Graphics Driver Developer
>>>> SUSE Software Solutions Germany GmbH
>>>> Maxfeldstr. 5, 90409 Nürnberg, Germany
>>>> (HRB 36809, AG Nürnberg)
>>>> Geschäftsführer: Ivo Totev
>>>
>>
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/intel-gfx/attachments/20230203/c02e4696/attachment-0001.sig>
More information about the Intel-gfx
mailing list