On 20/03/2019 08:57, Tomi Valkeinen wrote: > On 19/03/2019 20:18, Andrey Smirnov wrote: > >> TC358767 has two GPIO pins that can be used for HPD signal. I think >> instead of hardcoding GPIO0 here it would be more flexible to expose >> boths gpios as a gpiochip and use gpiolib API to query the value of >> HPD as well as use "hpd-gpios" binidng in DT to select which input to >> use. >> >> Another argument in favour of this solution is that Toshiba's FAEs (at >> least some) recommend thier customers to connect HPD signal to SoC's >> GPIOs and bypass TC358767 entirely. Their reasoning being that >> TC358767 implements a generic GPIO contoller and there's no advantage >> in going through TC358767 if you could use your "normal" GPIOs. >> >> Using gpiolib API would allow us to handle both usecase with the same >> code. >> >> Lastly, by treating "hpd-gpios" as an optional property, we can >> preserve old driver behaviour regardless if it is connected to DP or >> eDP panel. Not saying that this is really worth doing, just pointing >> out that this option would be on the table as well. > > I think that's a good point. > > There's one thing that TC358767 offers, which may not be available on > most generic GPIO controllers, though: it can detect short (programmable > length) pulses, thus it's possible to easily implement the DisplayPort > IRQ mechanism. I'm not sure if it's possible to implement reliable DP > IRQ detection without HW support. > > Still, I think using standard gpios makes sense.
After implementing the gpiochip (it works), I started to wonder... If TC358767 is used as a gpio expander, for whatever purpose, outside the TC358767 driver, then obviously we need the gpiochip driver. But I don't think anyone needs that. Then we have two cases 1) HPD connected to TC358767, 2) HPD goes directly to the SoC, or worded differently, HPD is handled by something else than TC358767. 1) was implemented in this current patch, and there's no real benefit with the gpiochip. It's somewhat confusing that the driver provides a gpiochip which the same driver then uses, for its internal functionality. 2) should actually not involve TC358767 driver at all as it's totally outside TC358767. If the HPD goes from the DP connector to the SoC, we should have the DP connector driver handle it. Currently that connector is in the TC358767 driver, but it should really be separated. So... Obviously what's missing from the current patch is that we need to be able to say which of the two GPIOs are used for the HPD (if any). But I'm debating with myself whether gpiochip here is a sane choice or not. Tomi -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
