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@xxxxxxxxxxxxxxx>
> > >>>
> > >>> 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@xxxxxxxxxxx>
>
> 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.
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
