From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758385AbcEFOnY (ORCPT ); Fri, 6 May 2016 10:43:24 -0400 Received: from mail-wm0-f41.google.com ([74.125.82.41]:37941 "EHLO mail-wm0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757922AbcEFOnW (ORCPT ); Fri, 6 May 2016 10:43:22 -0400 Date: Fri, 6 May 2016 16:43:18 +0200 From: Thierry Reding To: Noralf =?utf-8?Q?Tr=C3=B8nnes?= Cc: treding@nvidia.com, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Daniel Vetter Subject: Re: [PATCH 4/4] drm/panel: Add helper for simple panel connector Message-ID: <20160506144318.GA15076@ulmo.ba.sec> References: <1462454674-2246-1-git-send-email-noralf@tronnes.org> <1462454674-2246-5-git-send-email-noralf@tronnes.org> <20160506140347.GB4641@ulmo.ba.sec> <20160506140816.GD27098@phenom.ffwll.local> <20160506141532.GC4641@ulmo.ba.sec> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="zYM0uCDKw75PZbzx" Content-Disposition: inline In-Reply-To: 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 --zYM0uCDKw75PZbzx Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 06, 2016 at 04:34:08PM +0200, Noralf Tr=C3=B8nnes wrote: >=20 > Den 06.05.2016 16:15, skrev Thierry Reding: > > On Fri, May 06, 2016 at 04:08:16PM +0200, Daniel Vetter wrote: > > > On Fri, May 06, 2016 at 04:03:47PM +0200, Thierry Reding wrote: > > > > On Thu, May 05, 2016 at 03:24:34PM +0200, Noralf Tr=C3=B8nnes wrote: > > > > > Add function to create a simple connector for a panel. > > > > I'm not sure I see the usefulness of this. Typically you'd attach a > > > > panel to an encoder/connector, in which case you already have the > > > > connector. > > > >=20 > > > > Perhaps it would become more obvious why we need this if you posted > > > > patches that show where this is used? > > > The other helpers give you a simple drm pipeline with plane, crtc & > > > encoder all baked into on drm_simple_pipeline structure. The only thi= ng > > > variable you have to hook up to that is the drm_connector. And I thin= k for > > > dead-simple panels avoiding the basic boilerplate in that does indeed= make > > > some sense. > > Avoiding boilerplate is good, but I have a difficult time envisioning > > how you might want to use this. At the same time I'm asking myself how > > we know that this helper is any good if we haven't seen it used anywhere > > and actually see the boilerplate go away. >=20 > I pulled out the patches from the tinydrm patchset that would go > into drm core and helpers. I'm not very good at juggling many patches > around in various version and getting it right. > I'm doing development in the downstream Raspberry Pi repo > on 4.5 to get all the pi drivers, and then apply it on linux-next... >=20 > This is the tinydrm patch that will use it: > https://lists.freedesktop.org/archives/dri-devel/2016-April/104502.html >=20 > Extract: > +int tinydrm_display_pipe_init(struct tinydrm_device *tdev, > + const uint32_t *formats, unsigned int format_count) > +{ > + struct drm_device *dev =3D tdev->base; > + struct drm_connector *connector; > + int ret; > + > + connector =3D drm_simple_kms_panel_connector_create(dev, &tdev->pane= l, > + DRM_MODE_CONNECTOR_VIRTUAL); > + if (IS_ERR(connector)) > + return PTR_ERR(connector); > + > + ret =3D drm_simple_display_pipe_init(dev, &tdev->pipe, > + &tinydrm_display_pipe_funcs, > + formats, format_count, > + connector); > + > + return ret; > +} > +EXPORT_SYMBOL(tinydrm_display_pipe_init); >=20 > drm_simple_kms_panel_connector_create() changed name when Daniel > suggested it be moved to drm_panel.c or drm_panel_helper.c. Okay, that gives me a better understanding of where things are headed. That said, why would these devices even need to deal with drm_panel in the first place? Sounds to me like they are devices on some sort of control bus that you talk to directly. And they will represent essentially the panel with its built-in display controller. drm_panel on the other hand was designed as an interface between display controllers and panels, with the goal to let any display controller talk to any panel. While I'm sure you can support these types of simple panels using the drm_panel infrastructure I'm not sure it's necessary, since your driver will always talk to the panel directly, and hence the code to deal with the panel specifics could be part of the display pipe functions. Thierry --zYM0uCDKw75PZbzx Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAABCAAGBQJXLK1/AAoJEN0jrNd/PrOhsBkQAJDAWbsGlZ8UoNN90aeOma2T Cf3tQ2Z8JsqpeRKhAcYREYF4mduAKz9gqehG/pWyG3k05X1nu7ieWMy9fGJ8CcHf F/loZJJucfJu1AMD9VsmZHzsyrY9pcoTeP9UfQ3QA232wXDUUT9k4spDEVK2n34R 6S0rouRWKWLejC7cAQm7k1tOmOnxqJZhVjmdWPtjjVtLtyNZ0HGYZ91zJjv2Gz3m 0fyYPBXA3Ro7SxzXTqlzbCgu48z2bMOWyIhwF+dxeEUinvqcg3KIrfEQXdyM5Jxg NPSVgJpsQX5gQTp+GwfmOzrX5rR+emZ6uKshLqjkcoFCM+9IrDzlq7HyNl6f4Tys vpHqM6Q7tVSdQok7gXk7OjoPNO0ieO0sQlhO87jbBUxduCzz6X9XczTlSuaNLNr/ 7G+J/XXcQSHiW6wy/RP8wS6Ct2oBNyKZ4V6/zM5T3PS8SxGe3negNplFXVsiD4Hg wBf8UcSNx9XLeiSC+n60/lBQJcdcDX3h9iVRN2BGn8PrLbMDWbNWe5Ff31k9aap1 uJfFlAZDPVCPOZNsqZkD1xF0xXEWaGl7pJzd6Bc/IIo74UuxPiNm49+Tr4UcSqZS 594KTUhChiWitUhozvzULwTRdb6JOjAw9vjC15FLx42W0YkLlW+Akv26o+/mDW/1 O0ixpKIMFdBWmCMhj7Qu =Ylvf -----END PGP SIGNATURE----- --zYM0uCDKw75PZbzx--