Re: [PATCH v2 54/60] drm/omap: dss: Merge two disconnection helpers

2018-06-11 Thread Sebastian Reichel 
Hi,

On Sat, May 26, 2018 at 08:25:12PM +0300, Laurent Pinchart wrote:
> To simplify the pipeline disconnection handling merge the
> omapdss_device_disconnect() and omapdss_output_unset_device() functions.
> The device state check is now called for every device in the pipeline,
> extending this sanity check coverage.
> 
> There is no need to return an error from omapdss_device_disconnect()
> when the check fails, as omapdss_output_unset_device() used to do, given
> that we can't prevent disconnection due to device unbinding (the return
> value of omapdss_output_unset_device() is never checked in the current
> code for that reason).
> 
> Signed-off-by: Laurent Pinchart 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/gpu/drm/omapdrm/dss/base.c|  2 ++
>  drivers/gpu/drm/omapdrm/dss/dpi.c |  1 -
>  drivers/gpu/drm/omapdrm/dss/dsi.c |  1 -
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c   |  1 -
>  drivers/gpu/drm/omapdrm/dss/hdmi5.c   |  1 -
>  drivers/gpu/drm/omapdrm/dss/omapdss.h |  1 -
>  drivers/gpu/drm/omapdrm/dss/output.c  | 21 -
>  drivers/gpu/drm/omapdrm/dss/sdi.c |  1 -
>  drivers/gpu/drm/omapdrm/dss/venc.c|  1 -
>  9 files changed, 2 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/base.c 
> b/drivers/gpu/drm/omapdrm/dss/base.c
> index 3afb3d574a7b..fbb600d49ad2 100644
> --- a/drivers/gpu/drm/omapdrm/dss/base.c
> +++ b/drivers/gpu/drm/omapdrm/dss/base.c
> @@ -236,6 +236,8 @@ void omapdss_device_disconnect(struct omap_dss_device 
> *src,
>   src->dst = NULL;
>   }
>  
> + WARN_ON(dst->state != OMAP_DSS_DISPLAY_DISABLED);
> +
>   if (dst->driver)
>   dst->driver->disconnect(src, dst);
>   else
> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c 
> b/drivers/gpu/drm/omapdrm/dss/dpi.c
> index 831de09770a3..4cf397c9300e 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> @@ -633,7 +633,6 @@ static void dpi_disconnect(struct omap_dss_device *src,
>  struct omap_dss_device *dst)
>  {
>   omapdss_device_disconnect(dst, dst->next);
> - omapdss_output_unset_device(dst);
>  
>   dss_mgr_disconnect(dst);
>  }
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
> b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 53a32e20c3bd..bc470baa4f5c 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -4903,7 +4903,6 @@ static void dsi_disconnect(struct omap_dss_device *src,
>  struct omap_dss_device *dst)
>  {
>   omapdss_device_disconnect(dst, dst->next);
> - omapdss_output_unset_device(dst);
>  
>   dss_mgr_disconnect(dst);
>  }
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c 
> b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> index 22f8b74f5bf5..6616530d5fe6 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -450,7 +450,6 @@ static void hdmi_disconnect(struct omap_dss_device *src,
>   struct omap_dss_device *dst)
>  {
>   omapdss_device_disconnect(dst, dst->next);
> - omapdss_output_unset_device(dst);
>  
>   dss_mgr_disconnect(dst);
>  }
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c 
> b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> index d8592d02a58d..f7e15edc05fc 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> @@ -455,7 +455,6 @@ static void hdmi_disconnect(struct omap_dss_device *src,
>   struct omap_dss_device *dst)
>  {
>   omapdss_device_disconnect(dst, dst->next);
> - omapdss_output_unset_device(dst);
>  
>   dss_mgr_disconnect(dst);
>  }
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
> b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index 7add73a5479a..672761b5b017 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -518,7 +518,6 @@ int omap_dss_get_num_overlays(void);
>  #define for_each_dss_output(d) \
>   while ((d = omapdss_device_get_next(d, false, true)) != NULL)
>  int omapdss_output_validate(struct omap_dss_device *out);
> -int omapdss_output_unset_device(struct omap_dss_device *out);
>  
>  typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
>  int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
> diff --git a/drivers/gpu/drm/omapdrm/dss/output.c 
> b/drivers/gpu/drm/omapdrm/dss/output.c
> index be544dd48bf4..2da480be918d 100644
> --- a/drivers/gpu/drm/omapdrm/dss/output.c
> +++ b/drivers/gpu/drm/omapdrm/dss/output.c
> @@ -24,8 +24,6 @@
>  #include "dss.h"
>  #include "omapdss.h"
>  
> -static DEFINE_MUTEX(output_lock);
> -
>  int omapdss_output_validate(struct omap_dss_device *out)
>  {
>   if (out->next && out->output_type != out->next->type) {
> @@ -37,25 +35,6 @@ int omapdss_output_validate(struct omap_dss_device *out)
>  }
>  EXPORT_SYMBOL(omapdss_output_validate);
>  
> -int

[PATCH v2 54/60] drm/omap: dss: Merge two disconnection helpers

2018-05-26 Thread Laurent Pinchart 
To simplify the pipeline disconnection handling merge the
omapdss_device_disconnect() and omapdss_output_unset_device() functions.
The device state check is now called for every device in the pipeline,
extending this sanity check coverage.

There is no need to return an error from omapdss_device_disconnect()
when the check fails, as omapdss_output_unset_device() used to do, given
that we can't prevent disconnection due to device unbinding (the return
value of omapdss_output_unset_device() is never checked in the current
code for that reason).

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/omapdrm/dss/base.c|  2 ++
 drivers/gpu/drm/omapdrm/dss/dpi.c |  1 -
 drivers/gpu/drm/omapdrm/dss/dsi.c |  1 -
 drivers/gpu/drm/omapdrm/dss/hdmi4.c   |  1 -
 drivers/gpu/drm/omapdrm/dss/hdmi5.c   |  1 -
 drivers/gpu/drm/omapdrm/dss/omapdss.h |  1 -
 drivers/gpu/drm/omapdrm/dss/output.c  | 21 -
 drivers/gpu/drm/omapdrm/dss/sdi.c |  1 -
 drivers/gpu/drm/omapdrm/dss/venc.c|  1 -
 9 files changed, 2 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/base.c 
b/drivers/gpu/drm/omapdrm/dss/base.c
index 3afb3d574a7b..fbb600d49ad2 100644
--- a/drivers/gpu/drm/omapdrm/dss/base.c
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -236,6 +236,8 @@ void omapdss_device_disconnect(struct omap_dss_device *src,
src->dst = NULL;
}
 
+   WARN_ON(dst->state != OMAP_DSS_DISPLAY_DISABLED);
+
if (dst->driver)
dst->driver->disconnect(src, dst);
else
diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c 
b/drivers/gpu/drm/omapdrm/dss/dpi.c
index 831de09770a3..4cf397c9300e 100644
--- a/drivers/gpu/drm/omapdrm/dss/dpi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
@@ -633,7 +633,6 @@ static void dpi_disconnect(struct omap_dss_device *src,
   struct omap_dss_device *dst)
 {
omapdss_device_disconnect(dst, dst->next);
-   omapdss_output_unset_device(dst);
 
dss_mgr_disconnect(dst);
 }
diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c 
b/drivers/gpu/drm/omapdrm/dss/dsi.c
index 53a32e20c3bd..bc470baa4f5c 100644
--- a/drivers/gpu/drm/omapdrm/dss/dsi.c
+++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
@@ -4903,7 +4903,6 @@ static void dsi_disconnect(struct omap_dss_device *src,
   struct omap_dss_device *dst)
 {
omapdss_device_disconnect(dst, dst->next);
-   omapdss_output_unset_device(dst);
 
dss_mgr_disconnect(dst);
 }
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c 
b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index 22f8b74f5bf5..6616530d5fe6 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -450,7 +450,6 @@ static void hdmi_disconnect(struct omap_dss_device *src,
struct omap_dss_device *dst)
 {
omapdss_device_disconnect(dst, dst->next);
-   omapdss_output_unset_device(dst);
 
dss_mgr_disconnect(dst);
 }
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c 
b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
index d8592d02a58d..f7e15edc05fc 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
@@ -455,7 +455,6 @@ static void hdmi_disconnect(struct omap_dss_device *src,
struct omap_dss_device *dst)
 {
omapdss_device_disconnect(dst, dst->next);
-   omapdss_output_unset_device(dst);
 
dss_mgr_disconnect(dst);
 }
diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h 
b/drivers/gpu/drm/omapdrm/dss/omapdss.h
index 7add73a5479a..672761b5b017 100644
--- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
+++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
@@ -518,7 +518,6 @@ int omap_dss_get_num_overlays(void);
 #define for_each_dss_output(d) \
while ((d = omapdss_device_get_next(d, false, true)) != NULL)
 int omapdss_output_validate(struct omap_dss_device *out);
-int omapdss_output_unset_device(struct omap_dss_device *out);
 
 typedef void (*omap_dispc_isr_t) (void *arg, u32 mask);
 int omap_dispc_register_isr(omap_dispc_isr_t isr, void *arg, u32 mask);
diff --git a/drivers/gpu/drm/omapdrm/dss/output.c 
b/drivers/gpu/drm/omapdrm/dss/output.c
index be544dd48bf4..2da480be918d 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -24,8 +24,6 @@
 #include "dss.h"
 #include "omapdss.h"
 
-static DEFINE_MUTEX(output_lock);
-
 int omapdss_output_validate(struct omap_dss_device *out)
 {
if (out->next && out->output_type != out->next->type) {
@@ -37,25 +35,6 @@ int omapdss_output_validate(struct omap_dss_device *out)
 }
 EXPORT_SYMBOL(omapdss_output_validate);
 
-int omapdss_output_unset_device(struct omap_dss_device *out)
-{
-   int r = 0;
-
-   mutex_lock(_lock);
-
-   if (out->dst->state != OMAP_DSS_DISPLAY_DISABLED) {
-   dev_err(out->dev,
-   "device %s is not disabled, cannot unset device\n",
-