From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Anholt Subject: Re: -EPROBE_DEFER and failed DSI panel probe Date: Fri, 17 Nov 2017 17:16:07 -0800 Message-ID: <87wp2oxu7c.fsf@anholt.net> References: <87k1yr9tkn.fsf@anholt.net> <87fu9eq899.fsf@anholt.net> <1a3da7c2-18f3-ef8a-ab50-66bfc88e3bf9@samsung.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0106132768==" Return-path: Received: from anholt.net (anholt.net [50.246.234.109]) by gabe.freedesktop.org (Postfix) with ESMTP id CB0F56EAD6 for ; Sat, 18 Nov 2017 01:16:10 +0000 (UTC) In-Reply-To: <1a3da7c2-18f3-ef8a-ab50-66bfc88e3bf9@samsung.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Andrzej Hajda , dri-devel@lists.freedesktop.org List-Id: dri-devel@lists.freedesktop.org --===============0106132768== Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Andrzej Hajda writes: > On 16.11.2017 21:27, Eric Anholt wrote: >> Andrzej Hajda writes: >> >>> On 15.11.2017 21:26, Eric Anholt wrote: >>>> I'm happy to have the DSI panel finally working on VC4 (just waiting on >>>> https://lists.freedesktop.org/archives/dri-devel/2017-October/156407.h= tml), >>>> but now I've got another problem to solve. It would be great if I cou= ld >>>> include the DSI panel in our upstream DT, so that it automatically >>>> worked when you plugged one in. However, right now we return >>>> -EPROBE_DEFER during bind unless the panel has actually shown up. This >>>> means that if you don't have the panel actually connected, you get this >>>> sequence at startup: >>>> >>>> [ 10.719929] [drm] Initialized >>>> [ 10.829510] vc4_hdmi 3f902000.hdmi: vc4-hdmi-hifi <-> 3f902000.hdmi= mapping ok >>>> [ 10.844043] vc4-drm soc:gpu: bound 3f902000.hdmi (ops vc4_hdmi_ops = [vc4]) >>>> [ 10.848626] vc4-drm soc:gpu: bound 3f806000.vec (ops vc4_vec_ops [v= c4]) >>>> [ 10.850214] vc4-drm soc:gpu: failed to bind 3f700000.dsi (ops vc4_d= si_ops [vc4]): -517 >>>> [ 10.856559] vc4-drm soc:gpu: master bind failed: -517 >>>> >>>> [...] >>>> >>>> [ 10.967718] rpi_touchscreen 3-0045: Atmel I2C read failed: -6 >>>> >>>> Once the panel driver fails to probe, we never get asked to re-bind vc= 4, >>>> and drm_of_find_panel_or_bridge looks like it would just give us >>>> -EPROBE_DEFER again since the panel still wasn't registered. >>>> >>>> Does anyone have any suggestions for handling this? >>> I guess you should call component_add only when all required resources >>> are present(including panel), I suppose moving it to vc4_dsi_host_attach >>> should help. >> How can I decide when the panel driver has tried to probe and failed, >> versus not tried to probe yet? find_panel_or_bridge gives me >> -EPROBE_DEFER either way. >> >>> On the other side I am curious why EPROBE_DEFER from bind does not fail >>> probing of some component (the last one probed), with proper error >>> propagation it should cause defer_probing of one of the components or >>> master, and probe/bind should be retried after panel's probe. >> The panel probe failed, though, so there's no trigger to re-probe other >> drivers. > > OK, I misunderstood your problem. And after 2nd read I am not sure what > do you want to achieve exactly? > > Do you want to 'hotplug' DSI panel? Or just make it working with and > without the panel. In 2nd case other paths (HDMI) should still work, I > guess. Yeah, the second thing. I would like to use a single DT to describe a platform where the panel may or may not be present, so the panel driver (panel-raspberrypi-touchscreen.c) will throw an error from the I2C part of the probe or not instead of creating the DSI device and attaching the panel. > Lets assume that you are interested in the latter case. There could be > multiple solutions more or less hacky: > > 1. Use connector's HPD infrastructure to signal presence of the panel, > ie. in mipi_dsi::host_(attach|detach) callback you grab the panel and > change connector status to connected|disconnected and calls > drm_kms_helper_hotplug_event, it is done this way in exynos_dsi_host_atta= ch. This is basically what I had before all the review reworks, and the regression from this state is keeping me from merging the current driver downstream. This option is unfortunate because we'll have forced *some* output on at boot time, and then when the panel driver shows up later we don't resize the fbcon to the panel's size. > 2. Check for presence of the panel's driver before registering the bus, > if not present defer probing (but I am not sure if driver's registration > will trigger deferred probing, should be checked). > > This way if the panel's driver is present during mipi bus registration > and after that panel should be bound to the drivers, otherwise=C2=A0 it m= eans > probe failed. This solutions requires discovering panels compatible in > DSI driver. Quite complicated and very hacky. Restating to see if I understand: Defer all of VC4, including its MIPI host registration, until we see the panel driver loaded (how would one do that?). Then once we get reprobed with the MIPI driver present (would that even happen?), register our MIPI bus and continue on to component bind. In component bind, don't defer if we don't find the panel, because we know the panel driver had a chance to probe and attach right away when our bus showed up. > 3. In panel's probe attach the panel to the bus before hardware probe > and detach it if the physical panel is not present. This way dsi host > callbacks will be called and can be used to discover failed probes. If the I2C driver's probe fails, what ends up freeing the panel or DSI host driver that the I2C driver allocated when the module is unloaded? --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAEBCgAdFiEE/JuuFDWp9/ZkuCBXtdYpNtH8nugFAloPidgACgkQtdYpNtH8 nujuixAAi9Ek/VfQeBNZKbwER5jIPe3ghtGzxN1LwIMrZeQpNJfzHTHhL0hi0bpv EykR86O60DHslO6rtXbDk1Ekj0yP0GfHAtuH3U84RSznjlpRficiQrcmorNN8lGU Nn9ZRjXSTciRTd6ZgMkxG+CsAzZQBf5dhUGb3b20QqOGh9HfJhg337K+A21nBI+R 0ZlbbHeXhHkMI6lPQnsKPUPQ2snktMoUsirjQ2t/JPm93xMjqp659G3rh7/Niwzr sm2kLlnkBtdlcU4VaP03t7mexGiGo4Z55MX/V8i3q8ht65pmonSO412tXLeE44gm t5t8AZ0gAMuzKrs4okz6qhjlHuelY+zFf5EIoHas8vX/8PSiAPsrQxEx9UlukQRu TYv0UUZSmfR/XgMMpm+iP/KJoVZwCyhWZ06CAf4RdsbROWOoYEpKpPg4k5aSjWhk JD21/b7mT0Dy7w1z3b43+RHL5DA3WYyQgrvF5aVqRUftj0PXHUgaSyuvMJHSpj13 IStwDRIr1Gaj9IauR5JcSwEohZESfMf42Lfpm2bMumVMH/agA3FTRcBzfCQxbK+4 fhPozZ8CzDV4b55YOJGIj5bif1WT4apTLrME1PHZwhJyBzJDGiIpC+y3cmutC28C 80Y3L5VLrEzGahsLx/0/9O8XAs95GBtxRJZHGTWwHDPX0D9Ng0o= =gjCv -----END PGP SIGNATURE----- --=-=-=-- --===============0106132768== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVs IG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlz dHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== --===============0106132768==--