Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel

[Thierry Reding <thierry.reding@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1332 bytes --]

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.

Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]