From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752778AbcGLK72 (ORCPT ); Tue, 12 Jul 2016 06:59:28 -0400 Received: from mail-pf0-f181.google.com ([209.85.192.181]:32861 "EHLO mail-pf0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750816AbcGLK71 (ORCPT ); Tue, 12 Jul 2016 06:59:27 -0400 Date: Tue, 12 Jul 2016 12:59:22 +0200 From: Thierry Reding To: Emil Velikov , Vinay Simha BN , open list , "open list:DRM PANEL DRIVERS" , Archit Taneja Subject: Re: [PATCH v5 2/2] drm/panel: Add JDI LT070ME05000 WUXGA DSI Panel 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/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="liOOAslEiF7prFVr" Content-Disposition: inline In-Reply-To: <20160712103402.GA23520@phenom.ffwll.local> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --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--