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월 10일 10:06, YoungJun Cho wrote:
> On 07/10/2014 12:22 AM, Thierry Reding wrote:
>> On Tue, Jul 08, 2014 at 09:39:38AM +0900, YoungJun Cho wrote:
>>> To support LCD I80 interface, the DSI host calls this function
>>> to notify the panel tearing effect synchronization signal to
>>> the CRTC device manager to trigger to transfer video image.
>>>
>>> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
>>> Acked-by: Inki Dae <inki.dae@samsung.com>
>>> Acked-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> ---
>>>   drivers/gpu/drm/exynos/exynos_drm_dsi.c | 11 +++++++++++
>>>   include/drm/drm_mipi_dsi.h              |  7 +++++++
>>>   2 files changed, 18 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> index dad543a..76e34ca 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_dsi.c
>>> @@ -24,6 +24,7 @@
>>>   #include <video/mipi_display.h>
>>>   #include <video/videomode.h>
>>>
>>> +#include "exynos_drm_crtc.h"
>>>   #include "exynos_drm_drv.h"
>>>
>>>   /* returns true iff both arguments logically differs */
>>> @@ -1041,10 +1042,20 @@ static ssize_t
>>> exynos_dsi_host_transfer(struct mipi_dsi_host *host,
>>>       return (ret < 0) ? ret : xfer.rx_done;
>>>   }
>>>
>>> +static void exynos_dsi_host_pass_te(struct mipi_dsi_host *host)
>>> +{
>>> +    struct exynos_dsi *dsi = host_to_dsi(host);
>>> +    struct drm_encoder *encoder = dsi->encoder;
>>> +
>>> +    if (dsi->state & DSIM_STATE_ENABLED)
>>> +        exynos_drm_crtc_te_handler(encoder->crtc);
>>> +}
>>> +
>>>   static const struct mipi_dsi_host_ops exynos_dsi_ops = {
>>>       .attach = exynos_dsi_host_attach,
>>>       .detach = exynos_dsi_host_detach,
>>>       .transfer = exynos_dsi_host_transfer,
>>> +    .pass_te = exynos_dsi_host_pass_te,
>>>   };
>>>
>>>   static int exynos_dsi_poweron(struct exynos_dsi *dsi)
>>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>>> index 944f33f..3f21bea 100644
>>> --- a/include/drm/drm_mipi_dsi.h
>>> +++ b/include/drm/drm_mipi_dsi.h
>>> @@ -49,6 +49,12 @@ struct mipi_dsi_msg {
>>>    * @detach: detach DSI device from DSI host
>>>    * @transfer: send and/or receive DSI packet, return number of
>>> received bytes,
>>>    *           or error
>>> + * @pass_te: call the crtc te_handler() callback from DSI host.
>>> + *         The panel generates tearing effect synchronization signal
>>> between
>>> + *         MCU and FB to display video images. And the display
>>> controller
>>> + *         should trigger to transfer video image at this signal. So
>>> the panel
>>> + *         receives the TE IRQ, then calls this function to notify
>>> it to the
>>> + *         display controller.
>>>    */
>>>   struct mipi_dsi_host_ops {
>>>       int (*attach)(struct mipi_dsi_host *host,
>>> @@ -57,6 +63,7 @@ struct mipi_dsi_host_ops {
>>>                 struct mipi_dsi_device *dsi);
>>>       ssize_t (*transfer)(struct mipi_dsi_host *host,
>>>                   struct mipi_dsi_msg *msg);
>>> +    void (*pass_te)(struct mipi_dsi_host *host);
>>
>> I've objected to this particular change before and that objection still
>> stands. I don't see how this is related to DSI. It seems like an
>> implementation detail of this particular setup and I think it should be
>> handled differently (within the Exynos DSI controller implementation
>> possibly).
>>
> 
> Okay, I understand what you mean.
> As you know, this function is called by panel TE interrupt handler, so
> it could be accessed by panel.
> Do you have any good idea for panel to access exynos_drm_dsi directly
> without mipi_dis_host_ops?
> 

Mr. Cho, let's just use notifier callback for the meantime: fimd driver
registers a te handler to Exynos mipi dsi driver, and then Exynos mipi
dsi driver calls the te handler when te interrupt occurs from Panel device.

I think all we can consider for it is to use mipi_dsi_host_ops or core
framework - connector/encoder and crtc. However, it seems that it's not
really easy for we have a consensus with other maintainers until other
i80 users appear and they need common something for it.


Thanks,
Inki Dae

> Thank you.
> Best regards YJ
> 
>> Laurent also asked you to split this up into two patches, one for the
>> core part, the other for the Exynos driver parts, yet this patch
>> contains both changes.
>>
>> Thierry
>>
> 
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel