Re: [PATCH v2 52/60] drm/omap: dss: Move connection checks to omapdss_device_(dis)connect

2018-06-11 Thread Sebastian Reichel
Hi,

On Sat, May 26, 2018 at 08:25:10PM +0300, Laurent Pinchart wrote:
> When a DSS output is (dis)connected the omapdss_output_(un)set_device()
> function performs a sanity check to ensure that the output isn't already
> (dis)connected. The check is unnecessary as those situations should
> never happen, but can nonetheless be useful to catch driver bugs. To
> prepare for removal of the omapdss_output_(un)set_device() functions
> move the connection check to the omapdss_device_connect() function. The
> omapdss_device_disconnect() already contains a corresponding check.
> 
> Signed-off-by: Laurent Pinchart 
> ---

Reviewed-by: Sebastian Reichel 

-- Sebastian

>  drivers/gpu/drm/omapdrm/dss/base.c   |  1 +
>  drivers/gpu/drm/omapdrm/dss/output.c | 29 ++---
>  2 files changed, 3 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/base.c 
> b/drivers/gpu/drm/omapdrm/dss/base.c
> index 7f41c541397b..3afb3d574a7b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/base.c
> +++ b/drivers/gpu/drm/omapdrm/dss/base.c
> @@ -209,6 +209,7 @@ int omapdss_device_connect(struct dss_device *dss,
>   }
>  
>   if (src) {
> + WARN_ON(src->dst);
>   dst->src = src;
>   src->dst = dst;
>   }
> diff --git a/drivers/gpu/drm/omapdrm/dss/output.c 
> b/drivers/gpu/drm/omapdrm/dss/output.c
> index 2f7a019d059e..96d74218cf91 100644
> --- a/drivers/gpu/drm/omapdrm/dss/output.c
> +++ b/drivers/gpu/drm/omapdrm/dss/output.c
> @@ -29,61 +29,36 @@ static DEFINE_MUTEX(output_lock);
>  int omapdss_output_set_device(struct omap_dss_device *out,
>   struct omap_dss_device *dssdev)
>  {
> - int r;
> + int r = 0;
>  
>   mutex_lock(&output_lock);
>  
> - if (out->dst) {
> - dev_err(out->dev,
> - "output already has device %s connected to it\n",
> - out->dst->name);
> - r = -EINVAL;
> - goto err;
> - }
> -
>   if (out->output_type != dssdev->type) {
>   dev_err(out->dev, "output type and display type don't match\n");
>   r = -EINVAL;
> - goto err;
>   }
>  
>   mutex_unlock(&output_lock);
>  
> - return 0;
> -err:
> - mutex_unlock(&output_lock);
> -
>   return r;
>  }
>  EXPORT_SYMBOL(omapdss_output_set_device);
>  
>  int omapdss_output_unset_device(struct omap_dss_device *out)
>  {
> - int r;
> + int r = 0;
>  
>   mutex_lock(&output_lock);
>  
> - if (!out->dst) {
> - dev_err(out->dev,
> - "output doesn't have a device connected to it\n");
> - r = -EINVAL;
> - goto err;
> - }
> -
>   if (out->dst->state != OMAP_DSS_DISPLAY_DISABLED) {
>   dev_err(out->dev,
>   "device %s is not disabled, cannot unset device\n",
>   out->dst->name);
>   r = -EINVAL;
> - goto err;
>   }
>  
>   mutex_unlock(&output_lock);
>  
> - return 0;
> -err:
> - mutex_unlock(&output_lock);
> -
>   return r;
>  }
>  EXPORT_SYMBOL(omapdss_output_unset_device);
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel


[PATCH v2 52/60] drm/omap: dss: Move connection checks to omapdss_device_(dis)connect

2018-05-26 Thread Laurent Pinchart
When a DSS output is (dis)connected the omapdss_output_(un)set_device()
function performs a sanity check to ensure that the output isn't already
(dis)connected. The check is unnecessary as those situations should
never happen, but can nonetheless be useful to catch driver bugs. To
prepare for removal of the omapdss_output_(un)set_device() functions
move the connection check to the omapdss_device_connect() function. The
omapdss_device_disconnect() already contains a corresponding check.

Signed-off-by: Laurent Pinchart 
---
 drivers/gpu/drm/omapdrm/dss/base.c   |  1 +
 drivers/gpu/drm/omapdrm/dss/output.c | 29 ++---
 2 files changed, 3 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/base.c 
b/drivers/gpu/drm/omapdrm/dss/base.c
index 7f41c541397b..3afb3d574a7b 100644
--- a/drivers/gpu/drm/omapdrm/dss/base.c
+++ b/drivers/gpu/drm/omapdrm/dss/base.c
@@ -209,6 +209,7 @@ int omapdss_device_connect(struct dss_device *dss,
}
 
if (src) {
+   WARN_ON(src->dst);
dst->src = src;
src->dst = dst;
}
diff --git a/drivers/gpu/drm/omapdrm/dss/output.c 
b/drivers/gpu/drm/omapdrm/dss/output.c
index 2f7a019d059e..96d74218cf91 100644
--- a/drivers/gpu/drm/omapdrm/dss/output.c
+++ b/drivers/gpu/drm/omapdrm/dss/output.c
@@ -29,61 +29,36 @@ static DEFINE_MUTEX(output_lock);
 int omapdss_output_set_device(struct omap_dss_device *out,
struct omap_dss_device *dssdev)
 {
-   int r;
+   int r = 0;
 
mutex_lock(&output_lock);
 
-   if (out->dst) {
-   dev_err(out->dev,
-   "output already has device %s connected to it\n",
-   out->dst->name);
-   r = -EINVAL;
-   goto err;
-   }
-
if (out->output_type != dssdev->type) {
dev_err(out->dev, "output type and display type don't match\n");
r = -EINVAL;
-   goto err;
}
 
mutex_unlock(&output_lock);
 
-   return 0;
-err:
-   mutex_unlock(&output_lock);
-
return r;
 }
 EXPORT_SYMBOL(omapdss_output_set_device);
 
 int omapdss_output_unset_device(struct omap_dss_device *out)
 {
-   int r;
+   int r = 0;
 
mutex_lock(&output_lock);
 
-   if (!out->dst) {
-   dev_err(out->dev,
-   "output doesn't have a device connected to it\n");
-   r = -EINVAL;
-   goto err;
-   }
-
if (out->dst->state != OMAP_DSS_DISPLAY_DISABLED) {
dev_err(out->dev,
"device %s is not disabled, cannot unset device\n",
out->dst->name);
r = -EINVAL;
-   goto err;
}
 
mutex_unlock(&output_lock);
 
-   return 0;
-err:
-   mutex_unlock(&output_lock);
-
return r;
 }
 EXPORT_SYMBOL(omapdss_output_unset_device);
-- 
Regards,

Laurent Pinchart

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel