| Message ID | 1404779987-5337-6-git-send-email-yj44.cho@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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). 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
On 2014? 07? 10? 00:22, 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). I thought we had enough review but yes, I missed it. It seems better to handle pass_te callback within Exynos side as of now. That could cause controversy. However, the obvious one is that te signal should be notified to display controller driver somehow but there is no a good way for it. Anyway, got it. Reverted. Thanks, Inki Dae > > 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 >
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? 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 >
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 >> > >
On 2014? 07? 10? 10:20, Inki Dae wrote: > 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. > Again, there was my missing point. there wouldn't be way that Exynos mipi dsi driver can receive te signal from panel without mipi_dsi_host_ops. So let's use remote-endpoint property of DSI device node. DSI driver can get te gpio from Panel node, and can register te irq handler in case of i80 Panel. After that, te signal could be notified from te interrupt handler of DSI driver to FIMD driver through notifier callback registered already. Thanks, Inki Dae > 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 >
On Thu, Jul 10, 2014 at 10:06:07AM +0900, 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? I've gone through the DSI specification again and the only mention of the tearing effect is in section 8.12 "TE Signaling in DSI". That says the following: A Command Mode display module has its own timing controller and local frame buffer for display refresh. In some cases the host processor needs to be notified of timing events on the display module, e.g. the start of vertical blanking or similar timing information. In a traditional parallel-bus interface like DBI-2, a dedicated signal wire labeled TE (Tearing Effect) is provided to convey such timing information to the host processor. In a DSI system, the same information, with reasonably low latency, shall be transmitted from the display module to the host processor when requested, using the bidirectional Data Lane. My interpretation of that is that a DSI peripheral doesn't have a dedicated TE signal. Now the panel that you want to support here seems to have one, so I'm wondering if maybe it isn't a DSI panel at all but rather DBI. The specification goes into further detail about how to perform the TE reporting in DSI. Essentially it consists of giving the peripheral control of the bus via a BTA and then waiting for the peripheral to report back with the TE event. It would really help if somebody could find a datasheet for the panel so that we don't have to keep guessing what the actual interface is and how it's supposed to work. Thierry
Hi Thierry, Thank you for comment. On 07/10/2014 04:38 PM, Thierry Reding wrote: > On Thu, Jul 10, 2014 at 10:06:07AM +0900, 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? > > I've gone through the DSI specification again and the only mention of > the tearing effect is in section 8.12 "TE Signaling in DSI". That says > the following: > > A Command Mode display module has its own timing controller and > local frame buffer for display refresh. In some cases the host > processor needs to be notified of timing events on the display > module, e.g. the start of vertical blanking or similar timing > information. In a traditional parallel-bus interface like DBI-2, > a dedicated signal wire labeled TE (Tearing Effect) is provided > to convey such timing information to the host processor. In a > DSI system, the same information, with reasonably low latency, > shall be transmitted from the display module to the host > processor when requested, using the bidirectional Data Lane. > > My interpretation of that is that a DSI peripheral doesn't have a > dedicated TE signal. Now the panel that you want to support here seems > to have one, so I'm wondering if maybe it isn't a DSI panel at all but > rather DBI. Uhm, this panel is DSI panel right. It provides TE external pad to transfer TE pulse to host AP and it also provides TE relevant 3 DCS, so host AP could choose either of them. But I think it's better to use IRQ instead of polling method. As Inki commented before, I'll try to use remote-endpoint property of DSI device node in exynos DSIM driver and call FIMD notifier. Thank you. Best regards YJ > > The specification goes into further detail about how to perform the TE > reporting in DSI. Essentially it consists of giving the peripheral > control of the bus via a BTA and then waiting for the peripheral to > report back with the TE event. > > It would really help if somebody could find a datasheet for the panel so > that we don't have to keep guessing what the actual interface is and how > it's supposed to work. > > Thierry >
On Mon, Jul 14, 2014 at 06:22:39PM +0900, YoungJun Cho wrote: > Hi Thierry, > > Thank you for comment. > > On 07/10/2014 04:38 PM, Thierry Reding wrote: > >On Thu, Jul 10, 2014 at 10:06:07AM +0900, 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? > > > >I've gone through the DSI specification again and the only mention of > >the tearing effect is in section 8.12 "TE Signaling in DSI". That says > >the following: > > > > A Command Mode display module has its own timing controller and > > local frame buffer for display refresh. In some cases the host > > processor needs to be notified of timing events on the display > > module, e.g. the start of vertical blanking or similar timing > > information. In a traditional parallel-bus interface like DBI-2, > > a dedicated signal wire labeled TE (Tearing Effect) is provided > > to convey such timing information to the host processor. In a > > DSI system, the same information, with reasonably low latency, > > shall be transmitted from the display module to the host > > processor when requested, using the bidirectional Data Lane. > > > >My interpretation of that is that a DSI peripheral doesn't have a > >dedicated TE signal. Now the panel that you want to support here seems > >to have one, so I'm wondering if maybe it isn't a DSI panel at all but > >rather DBI. > > Uhm, this panel is DSI panel right. It provides TE external pad to transfer > TE pulse to host AP and it also provides TE relevant 3 DCS, so host AP could > choose either of them. > But I think it's better to use IRQ instead of polling method. According to the specification you don't have to rely on polling. You can simply pass control of the bus to the peripheral (via a BTA sequence) and then wait for the peripheral to signal TE. 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. 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. > 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. Thierry
Hi Thierry, On 07/14/2014 06:41 PM, Thierry Reding wrote: > On Mon, Jul 14, 2014 at 06:22:39PM +0900, YoungJun Cho wrote: >> Hi Thierry, >> >> Thank you for comment. >> >> On 07/10/2014 04:38 PM, Thierry Reding wrote: >>> On Thu, Jul 10, 2014 at 10:06:07AM +0900, 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? >>> >>> I've gone through the DSI specification again and the only mention of >>> the tearing effect is in section 8.12 "TE Signaling in DSI". That says >>> the following: >>> >>> A Command Mode display module has its own timing controller and >>> local frame buffer for display refresh. In some cases the host >>> processor needs to be notified of timing events on the display >>> module, e.g. the start of vertical blanking or similar timing >>> information. In a traditional parallel-bus interface like DBI-2, >>> a dedicated signal wire labeled TE (Tearing Effect) is provided >>> to convey such timing information to the host processor. In a >>> DSI system, the same information, with reasonably low latency, >>> shall be transmitted from the display module to the host >>> processor when requested, using the bidirectional Data Lane. >>> >>> My interpretation of that is that a DSI peripheral doesn't have a >>> dedicated TE signal. Now the panel that you want to support here seems >>> to have one, so I'm wondering if maybe it isn't a DSI panel at all but >>> rather DBI. >> >> Uhm, this panel is DSI panel right. It provides TE external pad to transfer >> TE pulse to host AP and it also provides TE relevant 3 DCS, so host AP could >> choose either of them. >> But I think it's better to use IRQ instead of polling method. > > According to the specification you don't have to rely on polling. You > can simply pass control of the bus to the peripheral (via a BTA > sequence) and then wait for the peripheral to signal TE. > I need to check that the Exynos DSIM driver supports this BTA sequence. > 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. > > 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? > >> 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). Thank you. Best regards YJ > > Thierry >
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. Thierry
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 >
Hi Inki, On 07/15/2014 11:34 AM, Inki Dae wrote: > 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. > The struct exynos_dsi has panel_node, which is valid by exynos_dsi_host_attach() is called from panel, we could use it instead of getting new remote-endpoint node. So after called exynos_dsi_host_attach(), the dsi driver could know that the panel supports mipi command mode or video mode, and if the panel is for mipi command mode one, dsi driver gets panel te gpio and registers its irq. I'll try. Thank you. Best regards YJ > Thanks, > Inki Dae > >> Thierry >> > >
On Wed, Jul 16, 2014 at 11:23:09AM +0900, YoungJun Cho wrote: > Hi Inki, > > On 07/15/2014 11:34 AM, Inki Dae wrote: > >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. > > > > The struct exynos_dsi has panel_node, which is valid by > exynos_dsi_host_attach() is called from panel, we could use it instead of > getting new remote-endpoint node. > > So after called exynos_dsi_host_attach(), the dsi driver could know that the > panel supports mipi command mode or video mode, > and if the panel is for mipi command mode one, dsi driver gets panel te gpio > and registers its irq. Why does the TE GPIO even need to be in the panel's device tree node? Wouldn't it make more sense for it to be in the DSIM node? After all that's what the GPIO is connected to, right? Well, at least logically if not physically. Thierry
Hi Thierry, On 07/16/2014 04:54 PM, Thierry Reding wrote: > On Wed, Jul 16, 2014 at 11:23:09AM +0900, YoungJun Cho wrote: >> Hi Inki, >> >> On 07/15/2014 11:34 AM, Inki Dae wrote: >>> 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. >>> >> >> The struct exynos_dsi has panel_node, which is valid by >> exynos_dsi_host_attach() is called from panel, we could use it instead of >> getting new remote-endpoint node. >> >> So after called exynos_dsi_host_attach(), the dsi driver could know that the >> panel supports mipi command mode or video mode, >> and if the panel is for mipi command mode one, dsi driver gets panel te gpio >> and registers its irq. > > Why does the TE GPIO even need to be in the panel's device tree node? > Wouldn't it make more sense for it to be in the DSIM node? After all > that's what the GPIO is connected to, right? Well, at least logically > if not physically. > I also agree that the GPIO in DT means the connection after all. But the panel provides the TE pin(this is obvious) and decides the DSIM mode(command or video). This TE pin is useless in video mode. So I think it's better for panel to decide and provide all related things. Thank you. Best regards YJ > Thierry >
On 2014? 07? 16? 19:12, YoungJun Cho wrote: > Hi Thierry, > > On 07/16/2014 04:54 PM, Thierry Reding wrote: >> On Wed, Jul 16, 2014 at 11:23:09AM +0900, YoungJun Cho wrote: >>> Hi Inki, >>> >>> On 07/15/2014 11:34 AM, Inki Dae wrote: >>>> 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. >>>> >>> >>> The struct exynos_dsi has panel_node, which is valid by >>> exynos_dsi_host_attach() is called from panel, we could use it >>> instead of >>> getting new remote-endpoint node. >>> >>> So after called exynos_dsi_host_attach(), the dsi driver could know >>> that the >>> panel supports mipi command mode or video mode, >>> and if the panel is for mipi command mode one, dsi driver gets panel >>> te gpio >>> and registers its irq. >> >> Why does the TE GPIO even need to be in the panel's device tree node? >> Wouldn't it make more sense for it to be in the DSIM node? After all >> that's what the GPIO is connected to, right? Well, at least logically >> if not physically. >> > > I also agree that the GPIO in DT means the connection after all. > > But the panel provides the TE pin(this is obvious) and decides the DSIM > mode(command or video). > This TE pin is useless in video mode. > > So I think it's better for panel to decide and provide all related things. > +1 device tree already connects them using endpoint node logically. And it wouldn't make sense for TE gpio pin to be in dsi node: TE pin belongs to panel device, not dsi. And if dsi node has te gpio property, where i80 based parallel panel's te pin should be in? Display controller? if TE pin is placed in panel node, and fimd and parallel panel or dsi and mipi panel are connected logically using device tree (graph), that case would be clear. Thanks, Inki Dae > Thank you. > Best regards YJ > >> Thierry >> > > -- > To unsubscribe from this list: send the line "unsubscribe devicetree" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
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); }; /**