Re: [PATCH 06/15] drm/dsi: Implement some standard DCS commands

[Andrzej Hajda <a.hajda@samsung.com>]

On 10/13/2014 12:16 PM, Thierry Reding wrote:
> From: YoungJun Cho <yj44.cho@samsung.com>
>
> Add helpers for the {enter,exit}_sleep_mode, set_display_{on,off} and
> set_tear_{on,off} DCS commands.
>
> Signed-off-by: YoungJun Cho <yj44.cho@samsung.com>
> [treding: kerneldoc and other minor cleanup]
> Signed-off-by: Thierry Reding <treding@nvidia.com>

The code below clearly shows that returning number of written bytes
by mipi_dsi_dcs_write is useless and causes only code bloat.
If mipi_dsi_dcs_write would return only error these function could be
written as static inlines in header file, for example:

static inline int mipi_dsi_dcs_enter_sleep_mode(struct mipi_dsi_device *dsi)
{
    return mipi_dsi_dcs_write(dsi, MIPI_DCS_ENTER_SLEEP_MODE, NULL, 0);
}

Regards
Andrzej

> ---
>  drivers/gpu/drm/drm_mipi_dsi.c | 118 +++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_mipi_dsi.h     |  19 +++++++
>  2 files changed, 137 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 7cd69273dbad..8bb38db861ec 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -506,6 +506,124 @@ ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
>  }
>  EXPORT_SYMBOL(mipi_dsi_dcs_read);
>  
> +/**
> + * mipi_dsi_dcs_enter_sleep_mode() - disable all unnecessary blocks inside the
> + *    display module except interface communication
> + * @dsi: DSI peripheral device
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int mipi_dsi_dcs_enter_sleep_mode(struct mipi_dsi_device *dsi)
> +{
> +	ssize_t err;
> +
> +	err = mipi_dsi_dcs_write(dsi, MIPI_DCS_ENTER_SLEEP_MODE, NULL, 0);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_enter_sleep_mode);
> +
> +/**
> + * mipi_dsi_dcs_exit_sleep_mode() - enable all blocks inside the display
> + *    module
> + * @dsi: DSI peripheral device
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int mipi_dsi_dcs_exit_sleep_mode(struct mipi_dsi_device *dsi)
> +{
> +	ssize_t err;
> +
> +	err = mipi_dsi_dcs_write(dsi, MIPI_DCS_EXIT_SLEEP_MODE, NULL, 0);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_exit_sleep_mode);
> +
> +/**
> + * mipi_dsi_dcs_set_display_off() - stop displaying the image data on the
> + *    display device
> + * @dsi: DSI peripheral device
> + *
> + * Return: 0 on success or a negative error code on failure.
> + */
> +int mipi_dsi_dcs_set_display_off(struct mipi_dsi_device *dsi)
> +{
> +	ssize_t err;
> +
> +	err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_OFF, NULL, 0);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_off);
> +
> +/**
> + * mipi_dsi_dcs_set_display_on() - start displaying the image data on the
> + *    display device
> + * @dsi: DSI peripheral device
> + *
> + * Return: 0 on success or a negative error code on failure
> + */
> +int mipi_dsi_dcs_set_display_on(struct mipi_dsi_device *dsi)
> +{
> +	ssize_t err;
> +
> +	err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_DISPLAY_ON, NULL, 0);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_display_on);
> +
> +/**
> + * mipi_dsi_dcs_set_tear_off() - turn off the display module's Tearing Effect
> + *    output signal on the TE signal line
> + * @dsi: DSI peripheral device
> + *
> + * Return: 0 on success or a negative error code on failure
> + */
> +int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi)
> +{
> +	ssize_t err;
> +
> +	err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_OFF, NULL, 0);
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_off);
> +
> +/**
> + * mipi_dsi_dcs_set_tear_on() - turn on the display module's Tearing Effect
> + *    output signal on the TE signal line.
> + * @dsi: DSI peripheral device
> + * @mode: the Tearing Effect Output Line mode
> + *
> + * Return: 0 on success or a negative error code on failure
> + */
> +int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
> +			     enum mipi_dsi_dcs_tear_mode mode)
> +{
> +	u8 value = mode;
> +	ssize_t err;
> +
> +	err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_ON, &value,
> +				 sizeof(value));
> +	if (err < 0)
> +		return err;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(mipi_dsi_dcs_set_tear_on);
> +
>  static int mipi_dsi_drv_probe(struct device *dev)
>  {
>  	struct mipi_dsi_driver *drv = to_mipi_dsi_driver(dev->driver);
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 92ca66306702..bde663725a58 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -140,12 +140,31 @@ ssize_t mipi_dsi_generic_write(struct mipi_dsi_device *dsi, const void *payload,
>  ssize_t mipi_dsi_generic_read(struct mipi_dsi_device *dsi, const void *params,
>  			      size_t num_params, void *data, size_t size);
>  
> +/**
> + * enum mipi_dsi_dcs_tear_mode - Tearing Effect Output Line mode
> + * @MIPI_DSI_DCS_TEAR_MODE_VBLANK: the TE output line consists of V-Blanking
> + *    information only
> + * @MIPI_DSI_DCS_TEAR_MODE_VHBLANK : the TE output line consists of both
> + *    V-Blanking and H-Blanking information
> + */
> +enum mipi_dsi_dcs_tear_mode {
> +	MIPI_DSI_DCS_TEAR_MODE_VBLANK,
> +	MIPI_DSI_DCS_TEAR_MODE_VHBLANK,
> +};
> +
>  ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi,
>  				  const void *data, size_t len);
>  ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
>  			   const void *data, size_t len);
>  ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
>  			  size_t len);
> +int mipi_dsi_dcs_enter_sleep_mode(struct mipi_dsi_device *dsi);
> +int mipi_dsi_dcs_exit_sleep_mode(struct mipi_dsi_device *dsi);
> +int mipi_dsi_dcs_set_display_off(struct mipi_dsi_device *dsi);
> +int mipi_dsi_dcs_set_display_on(struct mipi_dsi_device *dsi);
> +int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi);
> +int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
> +			     enum mipi_dsi_dcs_tear_mode mode);
>  
>  /**
>   * struct mipi_dsi_driver - DSI driver