Re: [PATCH v5 05/14] drm/exynos: dsi: add pass TE host ops to support LCD I80 interface

[Inki Dae <inki.dae@samsung.com>]

On 2014년 07월 14일 20:03, Thierry Reding wrote:
> On Mon, Jul 14, 2014 at 07:45:28PM +0900, YoungJun Cho wrote:
>> On 07/14/2014 06:41 PM, Thierry Reding wrote:
> [...]
>>> That said, I've been doing some research and it seems like we have a
>>> somewhat similar feature on Tegra. What happens there is that there are
>>> three GPIO pins that can be repurposed for TE signalling. But as opposed
>>> to using them as interrupts the display controller can be configured to
>>> use them, upon which it will automatically handle the TE signal by
>>> sending the next frame.
>>
>> Could you explain more detail how the Tegra display controller could be
>> configured with this GPIO pins?
>> I have no idea except that the display controller registers this GPIO as an
>> IRQ.
> 
> On Tegra the display controller has a special register that can be
> programmed to use one of the three GPIOs as TE signal. Then the display
> controller can be configured in one-shot (non-continuous) mode, which
> means that software needs to explicitly set a trigger bit to tell the
> display controller to send a new frame. If TE signalling is enabled,
> then the display controller will not immediately send a new frame when
> triggered but wait for signalling of this GPIO.
> 
>>> So we have at least two very different implementations of this on two
>>> different SoCs. Further the specification explicitly recommends using
>>> the BTA sequence and DSI protocol to wait for TE. So I still think that
>>> controllers that provide an additional, non-spec compliant method to
>>> signal TE should handle it separately rather than within DSI. Otherwise
>>> we essentially need to make the DSI "core" aware of all these quirks,
>>> and I'd rather avoid that.
>>
>> You mean, the DSI specification guides to use BTA, so it's better to use
>> display controller rather than DSIM, right?
> 
> What I'm saying is that there's nothing about a side-band TE wire in the
> DSI spec. In fact the spec explicitly says that this mechanism of an
> external TE wire from older protocols (DBI) was replaced by the BTA
> sequence over the protocol.
> 
> Now, my understanding is that using the BTA sequence over the DSI
> protocol would introduce some latency and that forces some panel vendors
> to still provide a side-band TE wire even in DSI compliant panels. But
> since this is not part of the specification there is no standard way to
> do this (as evidenced by Tegra and Exynos). Therefore putting such
> functionality into the core DSI code is bad.
> 
> But that doesn't mean that you have to put this functionality into the
> display controller driver on Exynos. What I'm saying is that it should
> be handled by the SoC driver rather than the core. Where exactly
> probably depends on the particular case.
> 
>>>> As Inki commented before, I'll try to use remote-endpoint property of DSI
>>>> device node in exynos DSIM driver and call FIMD notifier.
>>>
>>> Sounds like it matches what I said above. I'm not a huge fan of
>>> notifiers, but if it works for you I suppose that's fine. The
>>> alternative would be to directly call a FIMD function, which is
>>> somewhat more explicit than a notifier.
>>
>> Yes, I also like explicit call, so I want to use dsi_host_ops and calls it
>> in panel. But there is an objection to use dis_host_ops, we think notifier
>> in exynos dsim for fimd(display controller).
> 
> There are other ways to explicitly call into the display controller. You
> could for example get access to the CRTC that DSIM is currently
> connected to (via exynos_dsi.encoder->crtc) and then cast that to a
> struct exynos_drm_crtc and call a function to trigger a new frame to be
> sent (for example exynos_drm_crtc_send_frame()). This assumes that you
> can safely cast struct drm_crtc * to struct exynos_drm_crtc *, but that
> shouldn't be a problem.
> 
> With the above, you could make the DSIM handle the TE signal interrupts
> and trigger the DC using the new exynos_drm_crtc_send_frame() function.
> 

It seems better than the use of notifier. Actually, original patch used
this way except TE event.
Mr. Cho, let's use remote-endpoint property and this way instead of
notifier.

Thanks,
Inki Dae

> Thierry
>