lkml.org 
[lkml]   [2016]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel
On Tue, Jul 12, 2016 at 12:34:02PM +0200, Daniel Vetter wrote:
> On Mon, Jul 11, 2016 at 02:29:37PM +0200, Thierry Reding wrote:
> > On Thu, Jun 16, 2016 at 06:02:53PM +0100, Emil Velikov wrote:
> > > On 16 June 2016 at 04:00, Vinay Simha BN <simhavcs@gmail.com> wrote:
> > [...]
> > > > +static int jdi_panel_disable(struct drm_panel *panel)
> > > > +{
> > > > + struct jdi_panel *jdi = to_jdi_panel(panel);
> > > > +
> > > > + if (!jdi->enabled)
> > > > + return 0;
> > > > +
> > > Thinking out loud:
> > >
> > > Thierry,
> > > Shouldn't we fold 'enabled' and 'prepared' in struct drm_panel and
> > > tweak the helpers respectively ? Is there any specific reason for
> > > keeping these in the drivers ?
> >
> > Yes, I think that would make sense eventually. It's clearly a recurring
> > pattern. Ideally nothing would be calling these functions more than once
> > and thereby making the checks unnecessary. In practice that may mean
> > that we need to put the variables and checks into the drm/panel core
> > because display drivers (as opposed to a sane core implementation) call
> > these. I suppose we could encourage proper usage by adding a couple of
> > WARNs here and there if expectations aren't met.
> >
> > I don't think doing this is terribly urgent because it's easy to rip out
> > of drivers once the drm/panel core supports it. And it's something that
> > we could even leave within drivers when the core supports it, so trivial
> > to remove one by one after the core patches have landed.
>
> As long as we have non-atomic drm drivers using this multiple
> enable/disable calls can happen. Atomic drivers should screw this up
> (ignoring a few misguided ones that mix atomic and legacy helpers in bad
> ways, but those are getting fixed).
>
> I think a good plan would be:
> 1. Move this tracking into drm panel helpers, ditch it from all drivers.
> 2. Add WARN_ON for multiple enables/disables, but only for DRIVER_ATOMIC.
>
> Makes sure we can remove this boilerplate, makes sure that atomic drivers
> are consistent, leaves existing drivers unharmed.

Yeah, that sounds like a reasonable plan.

Thierry
[unhandled content-type:application/pgp-signature]
\
 
 \ /
  Last update: 2016-07-12 13:41    [W:0.155 / U:0.168 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site