Re: [Freedreno] [DPU PATCH v2 1/2] drm/msm/dsi: check video mode engine status before waiting

2018-04-16 Thread Sean Paul 
On Mon, Apr 16, 2018 at 10:44:57AM -0700, abhin...@codeaurora.org wrote:
> Hi Sean
> 
> Thanks for reviewing.
> 
> Reply inline.
> 
> On 2018-04-16 10:07, Sean Paul wrote:
> > On Fri, Apr 13, 2018 at 03:04:48PM -0700, abhin...@codeaurora.org wrote:
> > > On 2018-04-13 14:10, abhin...@codeaurora.org wrote:
> > > > Hi Sean
> > > >
> > > > Thanks for the review.
> > > >
> > > > Reply inline.
> > > >
> > > > On 2018-04-13 13:26, Sean Paul wrote:
> > > > > On Tue, Apr 10, 2018 at 06:54:06PM -0700, Abhinav Kumar wrote:
> > > > > > Make sure the video mode engine is on before waiting
> > > > > > for the video done interrupt.
> > > > > >
> > > > > > Otherwise it leads to silent timeouts increasing display
> > > > > > turn ON time.
> > > > > >
> > > > > > Changes in v2:
> > > > > > - Replace pr_err with dev_err
> > > > > > - Changed error message
> > > > > >
> > > > > > Signed-off-by: Abhinav Kumar 
> > > > > > ---
> > > > > >  drivers/gpu/drm/msm/dsi/dsi_host.c | 15 +++
> > > > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > > > b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > > > index 7a03a94..5b7b290 100644
> > > > > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > > > @@ -173,6 +173,7 @@ struct msm_dsi_host {
> > > > > >
> > > > > > bool registered;
> > > > > > bool power_on;
> > > > > > +   bool enabled;
> > > > > > int irq;
> > > > > >  };
> > > > > >
> > > > > > @@ -986,13 +987,19 @@ static void dsi_set_tx_power_mode(int
> > > > > > mode, struct msm_dsi_host *msm_host)
> > > > > >
> > > > > >  static void dsi_wait4video_done(struct msm_dsi_host *msm_host)
> > > > > >  {
> > > > > > +   u32 ret = 0;
> > > > > > +   struct device *dev = _host->pdev->dev;
> > > > > > +
> > > > > > dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1);
> > > > > >
> > > > > > reinit_completion(_host->video_comp);
> > > > > >
> > > > > > -   wait_for_completion_timeout(_host->video_comp,
> > > > > > +   ret = wait_for_completion_timeout(_host->video_comp,
> > > > > > msecs_to_jiffies(70));
> > > > > >
> > > > > > +   if (ret <= 0)
> > > > > > +   dev_err(dev, "wait for video done timed out\n");
> > > > > > +
> > > > > > dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0);
> > > > > >  }
> > > > > >
> > > > > > @@ -1001,7 +1008,7 @@ static void dsi_wait4video_eng_busy(struct
> > > > > > msm_dsi_host *msm_host)
> > > > > > if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
> > > > > > return;
> > > > > >
> > > > > > -   if (msm_host->power_on) {
> > > > > > +   if (msm_host->power_on && msm_host->enabled) {
> > > > > > dsi_wait4video_done(msm_host);
> > > > > > /* delay 4 ms to skip BLLP */
> > > > > > usleep_range(2000, 4000);
> > > > > > @@ -2203,7 +2210,7 @@ int msm_dsi_host_enable(struct
> > > > > > mipi_dsi_host *host)
> > > > > >  *  pm_runtime_put_autosuspend(_host->pdev->dev);
> > > > > >  * }
> > > > > >  */
> > > > > > -
> > > > > > +   msm_host->enabled = true;
> > > > > > return 0;
> > > > > >  }
> > > > > >
> > > > > > @@ -2219,7 +2226,7 @@ int msm_dsi_host_disable(struct
> > > > > > mipi_dsi_host *host)
> > > > > >  * Reset to disable video engine so that we can send off cmd.
> > > > > >  */
> > > > > > dsi_sw_reset(msm_host);
> > > > > > -
> > > > > > +   msm_host->enabled = false;
> > > > >
> > > > > This should go at the start of the function. Also, it's unclear from
> > > > > this patch,
> > > > > but I assume this is protected by a lock?
> > > > >
> > > > > Sean
> > > > [Abhinav] Yes, will move this to the start.
> > > > No, there is no lock here but at this point doesnt need one.
> > > > The reason is that, this variable will be written to and read by the
> > > > same process
> > > > (suspend thread OR resume thread which sends the panel ON/OFF commands).
> > > > If we decide to expose other interfaces to send commands like debugfs
> > > > or sysfs and
> > > > introduce more threads, we will add the locking.
> > > [Abhinav] Correction to my prev comment, we do have the
> > > msm_host->cmd_mutex
> > > which will
> > > ensure this entire process is protected. That should suffice.
> > 
> > Ok, thanks for confirming. Could you also please split this patch into
> > the
> > wait4video_done ret fix and the ->enabled addition? The 2 seem mostly
> > unrelated.
> > 
> > Sean
> 
> They are quite related actually. So we were not able to catch this earlier
> because the wait was silently timing out. Hence along with the fix I wanted
> to change the ret to add the error log because such condition should not
> happen with the ->enabled fix. To signify that I clubbed them together.
> I can split them, if this still feels unrelated.

Thanks for the explanation, they're unrelated in the sense that you're fixing 2
things in

Re: [Freedreno] [DPU PATCH v2 1/2] drm/msm/dsi: check video mode engine status before waiting

2018-04-16 Thread abhinavk 

Hi Sean

Thanks for reviewing.

Reply inline.

On 2018-04-16 10:07, Sean Paul wrote:
On Fri, Apr 13, 2018 at 03:04:48PM -0700, abhin...@codeaurora.org 
wrote:

On 2018-04-13 14:10, abhin...@codeaurora.org wrote:
> Hi Sean
>
> Thanks for the review.
>
> Reply inline.
>
> On 2018-04-13 13:26, Sean Paul wrote:
> > On Tue, Apr 10, 2018 at 06:54:06PM -0700, Abhinav Kumar wrote:
> > > Make sure the video mode engine is on before waiting
> > > for the video done interrupt.
> > >
> > > Otherwise it leads to silent timeouts increasing display
> > > turn ON time.
> > >
> > > Changes in v2:
> > > - Replace pr_err with dev_err
> > > - Changed error message
> > >
> > > Signed-off-by: Abhinav Kumar 
> > > ---
> > >  drivers/gpu/drm/msm/dsi/dsi_host.c | 15 +++
> > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > index 7a03a94..5b7b290 100644
> > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > @@ -173,6 +173,7 @@ struct msm_dsi_host {
> > >
> > >  bool registered;
> > >  bool power_on;
> > > +bool enabled;
> > >  int irq;
> > >  };
> > >
> > > @@ -986,13 +987,19 @@ static void dsi_set_tx_power_mode(int
> > > mode, struct msm_dsi_host *msm_host)
> > >
> > >  static void dsi_wait4video_done(struct msm_dsi_host *msm_host)
> > >  {
> > > +u32 ret = 0;
> > > +struct device *dev = _host->pdev->dev;
> > > +
> > >  dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1);
> > >
> > >  reinit_completion(_host->video_comp);
> > >
> > > -wait_for_completion_timeout(_host->video_comp,
> > > +ret = wait_for_completion_timeout(_host->video_comp,
> > >  msecs_to_jiffies(70));
> > >
> > > +if (ret <= 0)
> > > +dev_err(dev, "wait for video done timed out\n");
> > > +
> > >  dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0);
> > >  }
> > >
> > > @@ -1001,7 +1008,7 @@ static void dsi_wait4video_eng_busy(struct
> > > msm_dsi_host *msm_host)
> > >  if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
> > >  return;
> > >
> > > -if (msm_host->power_on) {
> > > +if (msm_host->power_on && msm_host->enabled) {
> > >  dsi_wait4video_done(msm_host);
> > >  /* delay 4 ms to skip BLLP */
> > >  usleep_range(2000, 4000);
> > > @@ -2203,7 +2210,7 @@ int msm_dsi_host_enable(struct
> > > mipi_dsi_host *host)
> > >   *  pm_runtime_put_autosuspend(_host->pdev->dev);
> > >   * }
> > >   */
> > > -
> > > +msm_host->enabled = true;
> > >  return 0;
> > >  }
> > >
> > > @@ -2219,7 +2226,7 @@ int msm_dsi_host_disable(struct
> > > mipi_dsi_host *host)
> > >   * Reset to disable video engine so that we can send off cmd.
> > >   */
> > >  dsi_sw_reset(msm_host);
> > > -
> > > +msm_host->enabled = false;
> >
> > This should go at the start of the function. Also, it's unclear from
> > this patch,
> > but I assume this is protected by a lock?
> >
> > Sean
> [Abhinav] Yes, will move this to the start.
> No, there is no lock here but at this point doesnt need one.
> The reason is that, this variable will be written to and read by the
> same process
> (suspend thread OR resume thread which sends the panel ON/OFF commands).
> If we decide to expose other interfaces to send commands like debugfs
> or sysfs and
> introduce more threads, we will add the locking.
[Abhinav] Correction to my prev comment, we do have the 
msm_host->cmd_mutex

which will
ensure this entire process is protected. That should suffice.


Ok, thanks for confirming. Could you also please split this patch into 
the
wait4video_done ret fix and the ->enabled addition? The 2 seem mostly 
unrelated.


Sean


They are quite related actually. So we were not able to catch this 
earlier
because the wait was silently timing out. Hence along with the fix I 
wanted

to change the ret to add the error log because such condition should not
happen with the ->enabled fix. To signify that I clubbed them together.
I can split them, if this still feels unrelated.


> >
> >
> > >  return 0;
> > >  }
> > >
> > > --
> > > The Qualcomm Innovation Center, Inc. is a member of the Code
> > > Aurora Forum,
> > > a Linux Foundation Collaborative Project
> > >
> > > ___
> > > Freedreno mailing list
> > > Freedreno@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/freedreno
> --
> To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
> in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

___
Freedreno mailing list

Re: [Freedreno] [DPU PATCH v2 1/2] drm/msm/dsi: check video mode engine status before waiting

2018-04-16 Thread Sean Paul 
On Fri, Apr 13, 2018 at 03:04:48PM -0700, abhin...@codeaurora.org wrote:
> On 2018-04-13 14:10, abhin...@codeaurora.org wrote:
> > Hi Sean
> > 
> > Thanks for the review.
> > 
> > Reply inline.
> > 
> > On 2018-04-13 13:26, Sean Paul wrote:
> > > On Tue, Apr 10, 2018 at 06:54:06PM -0700, Abhinav Kumar wrote:
> > > > Make sure the video mode engine is on before waiting
> > > > for the video done interrupt.
> > > > 
> > > > Otherwise it leads to silent timeouts increasing display
> > > > turn ON time.
> > > > 
> > > > Changes in v2:
> > > > - Replace pr_err with dev_err
> > > > - Changed error message
> > > > 
> > > > Signed-off-by: Abhinav Kumar 
> > > > ---
> > > >  drivers/gpu/drm/msm/dsi/dsi_host.c | 15 +++
> > > >  1 file changed, 11 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > index 7a03a94..5b7b290 100644
> > > > --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> > > > @@ -173,6 +173,7 @@ struct msm_dsi_host {
> > > > 
> > > > bool registered;
> > > > bool power_on;
> > > > +   bool enabled;
> > > > int irq;
> > > >  };
> > > > 
> > > > @@ -986,13 +987,19 @@ static void dsi_set_tx_power_mode(int
> > > > mode, struct msm_dsi_host *msm_host)
> > > > 
> > > >  static void dsi_wait4video_done(struct msm_dsi_host *msm_host)
> > > >  {
> > > > +   u32 ret = 0;
> > > > +   struct device *dev = _host->pdev->dev;
> > > > +
> > > > dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1);
> > > > 
> > > > reinit_completion(_host->video_comp);
> > > > 
> > > > -   wait_for_completion_timeout(_host->video_comp,
> > > > +   ret = wait_for_completion_timeout(_host->video_comp,
> > > > msecs_to_jiffies(70));
> > > > 
> > > > +   if (ret <= 0)
> > > > +   dev_err(dev, "wait for video done timed out\n");
> > > > +
> > > > dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0);
> > > >  }
> > > > 
> > > > @@ -1001,7 +1008,7 @@ static void dsi_wait4video_eng_busy(struct
> > > > msm_dsi_host *msm_host)
> > > > if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
> > > > return;
> > > > 
> > > > -   if (msm_host->power_on) {
> > > > +   if (msm_host->power_on && msm_host->enabled) {
> > > > dsi_wait4video_done(msm_host);
> > > > /* delay 4 ms to skip BLLP */
> > > > usleep_range(2000, 4000);
> > > > @@ -2203,7 +2210,7 @@ int msm_dsi_host_enable(struct
> > > > mipi_dsi_host *host)
> > > >  *  pm_runtime_put_autosuspend(_host->pdev->dev);
> > > >  * }
> > > >  */
> > > > -
> > > > +   msm_host->enabled = true;
> > > > return 0;
> > > >  }
> > > > 
> > > > @@ -2219,7 +2226,7 @@ int msm_dsi_host_disable(struct
> > > > mipi_dsi_host *host)
> > > >  * Reset to disable video engine so that we can send off cmd.
> > > >  */
> > > > dsi_sw_reset(msm_host);
> > > > -
> > > > +   msm_host->enabled = false;
> > > 
> > > This should go at the start of the function. Also, it's unclear from
> > > this patch,
> > > but I assume this is protected by a lock?
> > > 
> > > Sean
> > [Abhinav] Yes, will move this to the start.
> > No, there is no lock here but at this point doesnt need one.
> > The reason is that, this variable will be written to and read by the
> > same process
> > (suspend thread OR resume thread which sends the panel ON/OFF commands).
> > If we decide to expose other interfaces to send commands like debugfs
> > or sysfs and
> > introduce more threads, we will add the locking.
> [Abhinav] Correction to my prev comment, we do have the msm_host->cmd_mutex
> which will
> ensure this entire process is protected. That should suffice.

Ok, thanks for confirming. Could you also please split this patch into the
wait4video_done ret fix and the ->enabled addition? The 2 seem mostly unrelated.

Sean

> > > 
> > > 
> > > > return 0;
> > > >  }
> > > > 
> > > > --
> > > > The Qualcomm Innovation Center, Inc. is a member of the Code
> > > > Aurora Forum,
> > > > a Linux Foundation Collaborative Project
> > > > 
> > > > ___
> > > > Freedreno mailing list
> > > > Freedreno@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/freedreno
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-arm-msm"
> > in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [DPU PATCH v2 1/2] drm/msm/dsi: check video mode engine status before waiting

2018-04-13 Thread abhinavk 

Hi Sean

Thanks for the review.

Reply inline.

On 2018-04-13 13:26, Sean Paul wrote:

On Tue, Apr 10, 2018 at 06:54:06PM -0700, Abhinav Kumar wrote:

Make sure the video mode engine is on before waiting
for the video done interrupt.

Otherwise it leads to silent timeouts increasing display
turn ON time.

Changes in v2:
- Replace pr_err with dev_err
- Changed error message

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c

index 7a03a94..5b7b290 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -173,6 +173,7 @@ struct msm_dsi_host {

bool registered;
bool power_on;
+   bool enabled;
int irq;
 };

@@ -986,13 +987,19 @@ static void dsi_set_tx_power_mode(int mode, 
struct msm_dsi_host *msm_host)


 static void dsi_wait4video_done(struct msm_dsi_host *msm_host)
 {
+   u32 ret = 0;
+   struct device *dev = _host->pdev->dev;
+
dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1);

reinit_completion(_host->video_comp);

-   wait_for_completion_timeout(_host->video_comp,
+   ret = wait_for_completion_timeout(_host->video_comp,
msecs_to_jiffies(70));

+   if (ret <= 0)
+   dev_err(dev, "wait for video done timed out\n");
+
dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0);
 }

@@ -1001,7 +1008,7 @@ static void dsi_wait4video_eng_busy(struct 
msm_dsi_host *msm_host)

if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
return;

-   if (msm_host->power_on) {
+   if (msm_host->power_on && msm_host->enabled) {
dsi_wait4video_done(msm_host);
/* delay 4 ms to skip BLLP */
usleep_range(2000, 4000);
@@ -2203,7 +2210,7 @@ int msm_dsi_host_enable(struct mipi_dsi_host 
*host)

 *  pm_runtime_put_autosuspend(_host->pdev->dev);
 * }
 */
-
+   msm_host->enabled = true;
return 0;
 }

@@ -2219,7 +2226,7 @@ int msm_dsi_host_disable(struct mipi_dsi_host 
*host)

 * Reset to disable video engine so that we can send off cmd.
 */
dsi_sw_reset(msm_host);
-
+   msm_host->enabled = false;


This should go at the start of the function. Also, it's unclear from 
this patch,

but I assume this is protected by a lock?

Sean

[Abhinav] Yes, will move this to the start.
No, there is no lock here but at this point doesnt need one.
The reason is that, this variable will be written to and read by the 
same process

(suspend thread OR resume thread which sends the panel ON/OFF commands).
If we decide to expose other interfaces to send commands like debugfs or 
sysfs and

introduce more threads, we will add the locking.




return 0;
 }

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum,

a Linux Foundation Collaborative Project

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [Freedreno] [DPU PATCH v2 1/2] drm/msm/dsi: check video mode engine status before waiting

2018-04-13 Thread Sean Paul 
On Tue, Apr 10, 2018 at 06:54:06PM -0700, Abhinav Kumar wrote:
> Make sure the video mode engine is on before waiting
> for the video done interrupt.
> 
> Otherwise it leads to silent timeouts increasing display
> turn ON time.
> 
> Changes in v2:
> - Replace pr_err with dev_err
> - Changed error message
> 
> Signed-off-by: Abhinav Kumar 
> ---
>  drivers/gpu/drm/msm/dsi/dsi_host.c | 15 +++
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
> b/drivers/gpu/drm/msm/dsi/dsi_host.c
> index 7a03a94..5b7b290 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -173,6 +173,7 @@ struct msm_dsi_host {
>  
>   bool registered;
>   bool power_on;
> + bool enabled;
>   int irq;
>  };
>  
> @@ -986,13 +987,19 @@ static void dsi_set_tx_power_mode(int mode, struct 
> msm_dsi_host *msm_host)
>  
>  static void dsi_wait4video_done(struct msm_dsi_host *msm_host)
>  {
> + u32 ret = 0;
> + struct device *dev = _host->pdev->dev;
> +
>   dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1);
>  
>   reinit_completion(_host->video_comp);
>  
> - wait_for_completion_timeout(_host->video_comp,
> + ret = wait_for_completion_timeout(_host->video_comp,
>   msecs_to_jiffies(70));
>  
> + if (ret <= 0)
> + dev_err(dev, "wait for video done timed out\n");
> +
>   dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0);
>  }
>  
> @@ -1001,7 +1008,7 @@ static void dsi_wait4video_eng_busy(struct msm_dsi_host 
> *msm_host)
>   if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
>   return;
>  
> - if (msm_host->power_on) {
> + if (msm_host->power_on && msm_host->enabled) {
>   dsi_wait4video_done(msm_host);
>   /* delay 4 ms to skip BLLP */
>   usleep_range(2000, 4000);
> @@ -2203,7 +2210,7 @@ int msm_dsi_host_enable(struct mipi_dsi_host *host)
>*  pm_runtime_put_autosuspend(_host->pdev->dev);
>* }
>*/
> -
> + msm_host->enabled = true;
>   return 0;
>  }
>  
> @@ -2219,7 +2226,7 @@ int msm_dsi_host_disable(struct mipi_dsi_host *host)
>* Reset to disable video engine so that we can send off cmd.
>*/
>   dsi_sw_reset(msm_host);
> -
> + msm_host->enabled = false;

This should go at the start of the function. Also, it's unclear from this patch,
but I assume this is protected by a lock?

Sean


>   return 0;
>  }
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 
> ___
> Freedreno mailing list
> Freedreno@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/freedreno

-- 
Sean Paul, Software Engineer, Google / Chromium OS
___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

[Freedreno] [DPU PATCH v2 1/2] drm/msm/dsi: check video mode engine status before waiting

2018-04-10 Thread Abhinav Kumar 
Make sure the video mode engine is on before waiting
for the video done interrupt.

Otherwise it leads to silent timeouts increasing display
turn ON time.

Changes in v2:
- Replace pr_err with dev_err
- Changed error message

Signed-off-by: Abhinav Kumar 
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 15 +++
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c 
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 7a03a94..5b7b290 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -173,6 +173,7 @@ struct msm_dsi_host {
 
bool registered;
bool power_on;
+   bool enabled;
int irq;
 };
 
@@ -986,13 +987,19 @@ static void dsi_set_tx_power_mode(int mode, struct 
msm_dsi_host *msm_host)
 
 static void dsi_wait4video_done(struct msm_dsi_host *msm_host)
 {
+   u32 ret = 0;
+   struct device *dev = _host->pdev->dev;
+
dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1);
 
reinit_completion(_host->video_comp);
 
-   wait_for_completion_timeout(_host->video_comp,
+   ret = wait_for_completion_timeout(_host->video_comp,
msecs_to_jiffies(70));
 
+   if (ret <= 0)
+   dev_err(dev, "wait for video done timed out\n");
+
dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0);
 }
 
@@ -1001,7 +1008,7 @@ static void dsi_wait4video_eng_busy(struct msm_dsi_host 
*msm_host)
if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
return;
 
-   if (msm_host->power_on) {
+   if (msm_host->power_on && msm_host->enabled) {
dsi_wait4video_done(msm_host);
/* delay 4 ms to skip BLLP */
usleep_range(2000, 4000);
@@ -2203,7 +2210,7 @@ int msm_dsi_host_enable(struct mipi_dsi_host *host)
 *  pm_runtime_put_autosuspend(_host->pdev->dev);
 * }
 */
-
+   msm_host->enabled = true;
return 0;
 }
 
@@ -2219,7 +2226,7 @@ int msm_dsi_host_disable(struct mipi_dsi_host *host)
 * Reset to disable video engine so that we can send off cmd.
 */
dsi_sw_reset(msm_host);
-
+   msm_host->enabled = false;
return 0;
 }
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

___
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno