On Fri, Mar 23, 2018 at 03:58:06PM +0200, Ville Syrjälä wrote:
On Fri, Mar 23, 2018 at 02:37:23PM +0100, Noralf Trønnes wrote:
Den 23.03.2018 12.31, skrev Ville Syrjälä:
On Fri, Mar 23, 2018 at 12:43:58AM +0100, Noralf Trønnes wrote:
Den 22.03.2018 21.27, skrev Ville Syrjala:
From: Ville Syrjälä ville.syrjala@linux.intel.com
mipi_dbi_enable_flush() wants to call the fb->dirty() hook from the bowels of the .atomic_enable() hook. That prevents us from taking the plane mutex in fb->dirty() unless we also plumb down the acquire context.
Instead it seems simpler to split the fb->dirty() into a tinydrm specific lower level hook that can be called from mipi_dbi_enable_flush() and from a generic higher level tinydrm_fb_dirty() helper. As we don't have a tinydrm specific vfuncs table we'll just stick it into tinydrm_device directly for now.
The long term goal is to try and get rid of tinydrm.ko by moving stuff elsewhere as it's kind of a middle layer. So I'd really like to avoid adding a callback like this. Hopefully we can work out a solution based on my suggestion in the 'drm: Eliminate plane->fb/crtc usage for atomic drivers' thread.
I don't want to start redesigning the entire architecture at this point. That would also cause a bigger risk of regression and then we'd potentially have to try and revert the entire plane->fb thing, which would not be fun if any significant changes have occurred in the meantime.
If we do need a hook, I prefer that we add it to drm_simple_display_pipe_funcs.
If you have plans to redesign tinydrm anyway it seems to me that a temporary hook in tinydrm is may be a bit less intrusive than inflicting it on the simple_kms_helper.
Yes you're right of course, what was I thinking.
You missed out on one call site:
diff --git a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c index 11ae950b0fc9..7924eb59c2e1 100644 --- a/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c +++ b/drivers/gpu/drm/tinydrm/core/tinydrm-pipe.c @@ -124,11 +124,8 @@ void tinydrm_display_pipe_update(struct drm_simple_display_pipe *pipe, struct drm_framebuffer *fb = pipe->plane.state->fb; struct drm_crtc *crtc = &tdev->pipe.crtc;
- if (fb && (fb != old_state->fb)) { - pipe->plane.fb = fb; - if (fb->funcs->dirty) - fb->funcs->dirty(fb, NULL, 0, 0, NULL, 0); - } + if (fb && (fb != old_state->fb)) + tdev->fb_dirty(fb, NULL, 0, 0, NULL, 0);
if (crtc->state->event) { spin_lock_irq(&crtc->dev->event_lock);
With that fixed, series is:
Reviewed-by: Noralf Trønnes noralf@tronnes.org
Awesome. Thanks. And thanks for catching that extra dirty() call. I'll go and fix it up.
OK, I posted the fixed version.
Would you be interested in running some kind of smoke test on this before I push it? I'd hate to break things for you, and unfortunately (or maybe fortunately :) I don't have any hardware to test this.