Hi Am 23.01.23 um 13:20 schrieb Hogander, Jouni:
On Mon, 2023-01-23 at 12:38 +0100, Thomas Zimmermann wrote:Hi Am 23.01.23 um 08:44 schrieb Jouni Högander:After disconnecting damage worker from update logic it's left to fbdev emulation implementation to have fb_dirty function. Currently intel fbdev doesn't have it. This is causing problems to features (PSR, FBC, DRRS) relying on dirty callback. Implement simple fb_dirty callback to deliver notifications about updates in fb console. v4: Add proper Fixes tag and modify commit message v3: Check damage clip v2: Improved commit message and added Fixes tag Fixes: f231af498c29 ("drm/fb-helper: Disconnect damage worker from update logic") Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> Cc: Thomas Zimmermann <tzimmermann@xxxxxxx> Cc: Jani Nikula <jani.nikula@xxxxxxxxx> Signed-off-by: Jouni Högander <jouni.hogander@xxxxxxxxx> --- drivers/gpu/drm/i915/display/intel_fbdev.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c index 19f3b5d92a55..d39db8050c69 100644 --- a/drivers/gpu/drm/i915/display/intel_fbdev.c +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c @@ -321,8 +321,20 @@ static int intelfb_create(struct drm_fb_helper *helper, return ret; }+static int intelfb_dirty(struct drm_fb_helper *helper, structdrm_clip_rect *clip) +{ + if (!(clip->x1 < clip->x2 && clip->y1 < clip->y2)) + return 0; + + if (helper->fb->funcs->dirty) + return helper->fb->funcs->dirty(helper->fb, NULL, 0, 0, clip, 1);Didn't I nack this approach already? You shouldn't set fb_dirty. The better solution was to implement i915-specific helpers for write, fillarea, copyarea and blit. Those should call intelfb_dirty() directly after performing the output. And IIRC you already implemented this.I have implemented such thing. I didn't took it as a nack back then. Last comment from you was: "if you go with fb_dirty, please implement the clipping test in your callback." and v3 was prepared to address that comment. My thinking was that this is only for fb console, but Ville Syrjälä commented that there is more than console so I sticked in this approach. So you think I should just drop idea of setting dirty callback and implement those i915-specific helpers?
Ah OK, so I remembered incorrectly.A few things have changed since my original comment and I worked on fbdev helper a bit. The thing is that fb_dirty will likely go away at some point (together with the rest of drm_fb_helper_funcs). IOW at some point, you'll need those i915 functions anyways. Not using fb_dirty now will safe that work later on.
Best regards Thomas
Best regards Thomas+ + return 0; +} + static const struct drm_fb_helper_funcs intel_fb_helper_funcs = { .fb_probe = intelfb_create, + .fb_dirty = intelfb_dirty, };static void intel_fbdev_destroy(struct intel_fbdev *ifbdev)-- 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
Attachment:
OpenPGP_signature
Description: OpenPGP digital signature
