From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Subject: Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel Date: Tue, 12 Jul 2016 12:59:22 +0200 Message-ID: <20160712105922.GA15685@ulmo.ba.sec> References: <1466046050-20588-1-git-send-email-simhavcs@gmail.com> <1466046050-20588-2-git-send-email-simhavcs@gmail.com> <20160711122937.GB14709@ulmo.ba.sec> <20160712103402.GA23520@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0115674037==" Return-path: Received: from mail-pf0-x22e.google.com (mail-pf0-x22e.google.com [IPv6:2607:f8b0:400e:c00::22e]) by gabe.freedesktop.org (Postfix) with ESMTPS id 39D686E4EA for ; Tue, 12 Jul 2016 10:59:27 +0000 (UTC) Received: by mail-pf0-x22e.google.com with SMTP id c2so6023585pfa.2 for ; Tue, 12 Jul 2016 03:59:27 -0700 (PDT) In-Reply-To: <20160712103402.GA23520@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Emil Velikov , Vinay Simha BN , open list , "open list:DRM PANEL DRIVERS" , Archit Taneja List-Id: dri-devel@lists.freedesktop.org --===============0115674037== Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="liOOAslEiF7prFVr" Content-Disposition: inline --liOOAslEiF7prFVr Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 wrote: > > [...] > > > > +static int jdi_panel_disable(struct drm_panel *panel) > > > > +{ > > > > + struct jdi_panel *jdi =3D to_jdi_panel(panel); > > > > + > > > > + if (!jdi->enabled) > > > > + return 0; > > > > + > > > Thinking out loud: > > >=20 > > > 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 ? > >=20 > > 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. > >=20 > > 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. >=20 > 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). >=20 > 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. >=20 > 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 --liOOAslEiF7prFVr Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXhM2HAAoJEN0jrNd/PrOhjbIP/jSnfyDBi9PECB1nipVriPMK krgqmr9/IY1mr0lp1/2r2S91uJj8IZeVLizkr5PVhB/umhkwELiSOF/Bt6vJsHNl xi4r7uX4H0QWSuyn7sGoe19PYG2i3sI7PEqY7UQfrJkrPh4FVyO82Dczpv/xvmSV 5sNbwYj32+r7gGvIjcb8iTyh+MUxiv1EPQK6eFXyotWek+eG76IVS9Af0QOYsPKm 7xYUT8l6NBXT2migLhBZ/pQ/y9TSdJDkgh21X6AHqGs4NhJLrYKFtPMXZSirjNKG mI0Vbb/Q5DU0Z5fd33icel471ltk5TYAYzoU28radw4N1lDFM6dFqblTDudrPG1k ybZhv8zNKtscWAVmZO+uNo7zIp5ViYHe0SvcGsVFPy2v9N753IHC1vkyU7gWKI3t IyNPgO1HsmlZ97ooQlQ0Zb9+ZfKPJRL7mBwFASZamH0nRrE0GhH1vMBqg3Va/+mO 4LyLlHBG0P2+aNC4K7zPnqWtWdTPf86ahNBz+igvOAVrfuB7cye9ie/cADUzF3sD caih4leiCJ50hAr/7DyLGbqnaBQx3FV2sK55HNa4rkkgiwohwkpFzDGSWA1ZKavT JB6j/BvdGfkgIoHCv8+ro++2pYlnTIE5vcx8f7nesXbvvZckLLVfDy705rG+5kb8 4ksmYWQeYuCLwO7x4MEQ =iAsI -----END PGP SIGNATURE----- --liOOAslEiF7prFVr-- --===============0115674037== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0115674037==--