Re: [PATCH 06/29] drm/omap: Remove connection checks from internal encoders .enable()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Dec 05, 2018 at 04:59:59PM +0200, Laurent Pinchart wrote:
> The internal encoders return an error from their .enable() handler when
> their are not connected to a dss manager. As the flag used is set and
> cleared in the connect and disconnect handlers, this effectively checks
> whether the omap_dss_device is connected.
> 
> The .enable() handler is called from code paths that access the dss
> devices chain from the display device, which is set to NULL when the
> device isn't connected, making it impossible to access the device in
> that case.
> 
> The safety check is thus not needed, remove it.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@xxxxxxxxxxxxx>

-- Sebastian

>  drivers/gpu/drm/omapdrm/dss/dpi.c     | 17 +----------------
>  drivers/gpu/drm/omapdrm/dss/dsi.c     | 20 ++------------------
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c   | 17 +----------------
>  drivers/gpu/drm/omapdrm/dss/hdmi5.c   | 17 +----------------
>  drivers/gpu/drm/omapdrm/dss/omapdss.h |  1 -
>  drivers/gpu/drm/omapdrm/dss/sdi.c     | 16 +---------------
>  drivers/gpu/drm/omapdrm/dss/venc.c    | 17 +----------------
>  7 files changed, 7 insertions(+), 98 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/dpi.c b/drivers/gpu/drm/omapdrm/dss/dpi.c
> index ca4f3c4c6318..0cf3b220e35f 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dpi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dpi.c
> @@ -386,12 +386,6 @@ static int dpi_display_enable(struct omap_dss_device *dssdev)
>  
>  	mutex_lock(&dpi->lock);
>  
> -	if (!out->dispc_channel_connected) {
> -		DSSERR("failed to enable display: no output/manager\n");
> -		r = -ENODEV;
> -		goto err_no_out_mgr;
> -	}
> -
>  	if (dpi->vdds_dsi_reg) {
>  		r = regulator_enable(dpi->vdds_dsi_reg);
>  		if (r)
> @@ -439,7 +433,6 @@ static int dpi_display_enable(struct omap_dss_device *dssdev)
>  	if (dpi->vdds_dsi_reg)
>  		regulator_disable(dpi->vdds_dsi_reg);
>  err_reg_enable:
> -err_no_out_mgr:
>  	mutex_unlock(&dpi->lock);
>  	return r;
>  }
> @@ -596,23 +589,15 @@ static int dpi_connect(struct omap_dss_device *src,
>  		       struct omap_dss_device *dst)
>  {
>  	struct dpi_data *dpi = dpi_get_data_from_dssdev(dst);
> -	int r;
>  
>  	dpi_init_pll(dpi);
>  
> -	r = omapdss_device_connect(dst->dss, dst, dst->next);
> -	if (r)
> -		return r;
> -
> -	dst->dispc_channel_connected = true;
> -	return 0;
> +	return omapdss_device_connect(dst->dss, dst, dst->next);
>  }
>  
>  static void dpi_disconnect(struct omap_dss_device *src,
>  			   struct omap_dss_device *dst)
>  {
> -	dst->dispc_channel_connected = false;
> -
>  	omapdss_device_disconnect(dst, dst->next);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/dsi.c b/drivers/gpu/drm/omapdrm/dss/dsi.c
> index 0a485c5b982e..180951eb161c 100644
> --- a/drivers/gpu/drm/omapdrm/dss/dsi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/dsi.c
> @@ -3753,19 +3753,13 @@ static int dsi_enable_video_output(struct omap_dss_device *dssdev, int channel)
>  {
>  	struct dsi_data *dsi = to_dsi_data(dssdev);
>  	int bpp = dsi_get_pixel_size(dsi->pix_fmt);
> -	struct omap_dss_device *out = &dsi->output;
>  	u8 data_type;
>  	u16 word_count;
>  	int r;
>  
> -	if (!out->dispc_channel_connected) {
> -		DSSERR("failed to enable display: no output/manager\n");
> -		return -ENODEV;
> -	}
> -
>  	r = dsi_display_init_dispc(dsi);
>  	if (r)
> -		goto err_init_dispc;
> +		return r;
>  
>  	if (dsi->mode == OMAP_DSS_DSI_VIDEO_MODE) {
>  		switch (dsi->pix_fmt) {
> @@ -3814,7 +3808,6 @@ static int dsi_enable_video_output(struct omap_dss_device *dssdev, int channel)
>  	}
>  err_pix_fmt:
>  	dsi_display_uninit_dispc(dsi);
> -err_init_dispc:
>  	return r;
>  }
>  
> @@ -4877,21 +4870,12 @@ static int dsi_get_clocks(struct dsi_data *dsi)
>  static int dsi_connect(struct omap_dss_device *src,
>  		       struct omap_dss_device *dst)
>  {
> -	int r;
> -
> -	r = omapdss_device_connect(dst->dss, dst, dst->next);
> -	if (r)
> -		return r;
> -
> -	dst->dispc_channel_connected = true;
> -	return 0;
> +	return omapdss_device_connect(dst->dss, dst, dst->next);
>  }
>  
>  static void dsi_disconnect(struct omap_dss_device *src,
>  			   struct omap_dss_device *dst)
>  {
> -	dst->dispc_channel_connected = false;
> -
>  	omapdss_device_disconnect(dst, dst->next);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> index aabdda394c9c..b6b44f07c74e 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -322,12 +322,6 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
>  
>  	mutex_lock(&hdmi->lock);
>  
> -	if (!dssdev->dispc_channel_connected) {
> -		DSSERR("failed to enable display: no output/manager\n");
> -		r = -ENODEV;
> -		goto err0;
> -	}
> -
>  	r = hdmi_power_on_full(hdmi);
>  	if (r) {
>  		DSSERR("failed to power on device\n");
> @@ -417,21 +411,12 @@ void hdmi4_core_disable(struct hdmi_core_data *core)
>  static int hdmi_connect(struct omap_dss_device *src,
>  			struct omap_dss_device *dst)
>  {
> -	int r;
> -
> -	r = omapdss_device_connect(dst->dss, dst, dst->next);
> -	if (r)
> -		return r;
> -
> -	dst->dispc_channel_connected = true;
> -	return 0;
> +	return omapdss_device_connect(dst->dss, dst, dst->next);
>  }
>  
>  static void hdmi_disconnect(struct omap_dss_device *src,
>  			    struct omap_dss_device *dst)
>  {
> -	dst->dispc_channel_connected = false;
> -
>  	omapdss_device_disconnect(dst, dst->next);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> index 9e8556f67a29..beef25703eab 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> @@ -330,12 +330,6 @@ static int hdmi_display_enable(struct omap_dss_device *dssdev)
>  
>  	mutex_lock(&hdmi->lock);
>  
> -	if (!dssdev->dispc_channel_connected) {
> -		DSSERR("failed to enable display: no output/manager\n");
> -		r = -ENODEV;
> -		goto err0;
> -	}
> -
>  	r = hdmi_power_on_full(hdmi);
>  	if (r) {
>  		DSSERR("failed to power on device\n");
> @@ -422,21 +416,12 @@ static void hdmi_core_disable(struct omap_hdmi *hdmi)
>  static int hdmi_connect(struct omap_dss_device *src,
>  			struct omap_dss_device *dst)
>  {
> -	int r;
> -
> -	r = omapdss_device_connect(dst->dss, dst, dst->next);
> -	if (r)
> -		return r;
> -
> -	dst->dispc_channel_connected = true;
> -	return 0;
> +	return omapdss_device_connect(dst->dss, dst, dst->next);
>  }
>  
>  static void hdmi_disconnect(struct omap_dss_device *src,
>  			    struct omap_dss_device *dst)
>  {
> -	dst->dispc_channel_connected = false;
> -
>  	omapdss_device_disconnect(dst, dst->next);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/omapdss.h b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> index 149a0f09adbc..4e1e58e85429 100644
> --- a/drivers/gpu/drm/omapdrm/dss/omapdss.h
> +++ b/drivers/gpu/drm/omapdrm/dss/omapdss.h
> @@ -440,7 +440,6 @@ struct omap_dss_device {
>  
>  	/* DISPC channel for this output */
>  	enum omap_channel dispc_channel;
> -	bool dispc_channel_connected;
>  
>  	/* output instance */
>  	enum omap_dss_output_id id;
> diff --git a/drivers/gpu/drm/omapdrm/dss/sdi.c b/drivers/gpu/drm/omapdrm/dss/sdi.c
> index b2fe2387037a..7de817c69913 100644
> --- a/drivers/gpu/drm/omapdrm/dss/sdi.c
> +++ b/drivers/gpu/drm/omapdrm/dss/sdi.c
> @@ -136,11 +136,6 @@ static int sdi_display_enable(struct omap_dss_device *dssdev)
>  	unsigned long fck;
>  	int r;
>  
> -	if (!sdi->output.dispc_channel_connected) {
> -		DSSERR("failed to enable display: no output/manager\n");
> -		return -ENODEV;
> -	}
> -
>  	r = regulator_enable(sdi->vdds_sdi_reg);
>  	if (r)
>  		goto err_reg_enable;
> @@ -251,21 +246,12 @@ static int sdi_check_timings(struct omap_dss_device *dssdev,
>  static int sdi_connect(struct omap_dss_device *src,
>  		       struct omap_dss_device *dst)
>  {
> -	int r;
> -
> -	r = omapdss_device_connect(dst->dss, dst, dst->next);
> -	if (r)
> -		return r;
> -
> -	dst->dispc_channel_connected = true;
> -	return 0;
> +	return omapdss_device_connect(dst->dss, dst, dst->next);
>  }
>  
>  static void sdi_disconnect(struct omap_dss_device *src,
>  			   struct omap_dss_device *dst)
>  {
> -	dst->dispc_channel_connected = false;
> -
>  	omapdss_device_disconnect(dst, dst->next);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/venc.c b/drivers/gpu/drm/omapdrm/dss/venc.c
> index fbe9d42dbdb6..bc9a3d52f34d 100644
> --- a/drivers/gpu/drm/omapdrm/dss/venc.c
> +++ b/drivers/gpu/drm/omapdrm/dss/venc.c
> @@ -531,12 +531,6 @@ static int venc_display_enable(struct omap_dss_device *dssdev)
>  
>  	mutex_lock(&venc->venc_lock);
>  
> -	if (!dssdev->dispc_channel_connected) {
> -		DSSERR("Failed to enable display: no output/manager\n");
> -		r = -ENODEV;
> -		goto err0;
> -	}
> -
>  	r = venc_power_on(venc);
>  	if (r)
>  		goto err0;
> @@ -687,21 +681,12 @@ static int venc_get_clocks(struct venc_device *venc)
>  static int venc_connect(struct omap_dss_device *src,
>  			struct omap_dss_device *dst)
>  {
> -	int r;
> -
> -	r = omapdss_device_connect(dst->dss, dst, dst->next);
> -	if (r)
> -		return r;
> -
> -	dst->dispc_channel_connected = true;
> -	return 0;
> +	return omapdss_device_connect(dst->dss, dst, dst->next);
>  }
>  
>  static void venc_disconnect(struct omap_dss_device *src,
>  			    struct omap_dss_device *dst)
>  {
> -	dst->dispc_channel_connected = false;
> -
>  	omapdss_device_disconnect(dst, dst->next);
>  }
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux