[01/14] drm/mipi-dsi: Add a mipi_dsi_dcs_write_seq() macro

  Many panel drivers define dsi_dcs_write_seq() and dsi_generic_write_seq()
macros to send DCS commands and generic write packets respectively, with
the payload specified as a list of parameters instead of using arrays.

There's already a macro for the former, introduced by commit 2a9e9daf75231
("drm/mipi-dsi: Introduce mipi_dsi_dcs_write_seq macro") so drivers can be
changed to use that. But there isn't one yet for the latter, let's add it.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 include/drm/drm_mipi_dsi.h | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
  

Hi Javier,

I love your patch! Perhaps something to improve:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v6.2-rc1 next-20221226]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Javier-Martinez-Canillas/drm-mipi-dsi-Add-a-mipi_dsi_dcs_write_seq-macro/20221228-100040
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20221228014757.3170486-2-javierm%40redhat.com
patch subject: [PATCH 01/14] drm/mipi-dsi: Add a mipi_dsi_dcs_write_seq() macro
reproduce:
        # https://github.com/intel-lab-lkp/linux/commit/6dbe3eb57c38eaa1be1271fe9563406472377dc7
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Javier-Martinez-Canillas/drm-mipi-dsi-Add-a-mipi_dsi_dcs_write_seq-macro/20221228-100040
        git checkout 6dbe3eb57c38eaa1be1271fe9563406472377dc7
        make menuconfig
        # enable CONFIG_COMPILE_TEST, CONFIG_WARN_MISSING_DOCUMENTS, CONFIG_WARN_ABI_ERRORS
        make htmldocs

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> ./include/drm/drm_mipi_dsi.h:314: warning: expecting prototype for mipi_dsi_generic_write(). Prototype was for mipi_dsi_generic_write_seq() instead

vim +314 ./include/drm/drm_mipi_dsi.h

   271	
   272	ssize_t mipi_dsi_dcs_write_buffer(struct mipi_dsi_device *dsi,
   273					  const void *data, size_t len);
   274	ssize_t mipi_dsi_dcs_write(struct mipi_dsi_device *dsi, u8 cmd,
   275				   const void *data, size_t len);
   276	ssize_t mipi_dsi_dcs_read(struct mipi_dsi_device *dsi, u8 cmd, void *data,
   277				  size_t len);
   278	int mipi_dsi_dcs_nop(struct mipi_dsi_device *dsi);
   279	int mipi_dsi_dcs_soft_reset(struct mipi_dsi_device *dsi);
   280	int mipi_dsi_dcs_get_power_mode(struct mipi_dsi_device *dsi, u8 *mode);
   281	int mipi_dsi_dcs_get_pixel_format(struct mipi_dsi_device *dsi, u8 *format);
   282	int mipi_dsi_dcs_enter_sleep_mode(struct mipi_dsi_device *dsi);
   283	int mipi_dsi_dcs_exit_sleep_mode(struct mipi_dsi_device *dsi);
   284	int mipi_dsi_dcs_set_display_off(struct mipi_dsi_device *dsi);
   285	int mipi_dsi_dcs_set_display_on(struct mipi_dsi_device *dsi);
   286	int mipi_dsi_dcs_set_column_address(struct mipi_dsi_device *dsi, u16 start,
   287					    u16 end);
   288	int mipi_dsi_dcs_set_page_address(struct mipi_dsi_device *dsi, u16 start,
   289					  u16 end);
   290	int mipi_dsi_dcs_set_tear_off(struct mipi_dsi_device *dsi);
   291	int mipi_dsi_dcs_set_tear_on(struct mipi_dsi_device *dsi,
   292				     enum mipi_dsi_dcs_tear_mode mode);
   293	int mipi_dsi_dcs_set_pixel_format(struct mipi_dsi_device *dsi, u8 format);
   294	int mipi_dsi_dcs_set_tear_scanline(struct mipi_dsi_device *dsi, u16 scanline);
   295	int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi,
   296						u16 brightness);
   297	int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
   298						u16 *brightness);
   299	
   300	/**
   301	 * mipi_dsi_generic_write - transmit data using a generic write packet
   302	 * @dsi: DSI peripheral device
   303	 * @seq: buffer containing the payload
   304	 */
   305	#define mipi_dsi_generic_write_seq(dsi, seq...) do {				\
   306			static const u8 d[] = { seq };					\
   307			struct device *dev = &dsi->dev;	\
   308			int ret;						\
   309			ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));	\
   310			if (ret < 0) {						\
   311				dev_err_ratelimited(dev, "transmit data failed: %d\n", ret); \
   312				return ret;						\
   313			}						\
 > 314		} while (0)
   315
  

Hi Javier.

On Wed, Dec 28, 2022 at 02:47:44AM +0100, Javier Martinez Canillas wrote:
> Many panel drivers define dsi_dcs_write_seq() and dsi_generic_write_seq()
> macros to send DCS commands and generic write packets respectively, with
> the payload specified as a list of parameters instead of using arrays.
> 
> There's already a macro for the former, introduced by commit 2a9e9daf75231
> ("drm/mipi-dsi: Introduce mipi_dsi_dcs_write_seq macro") so drivers can be
> changed to use that. But there isn't one yet for the latter, let's add it.
> 
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>  include/drm/drm_mipi_dsi.h | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
> index 20b21b577dea..c7c458131ba1 100644
> --- a/include/drm/drm_mipi_dsi.h
> +++ b/include/drm/drm_mipi_dsi.h
> @@ -297,6 +297,22 @@ int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi,
>  int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
>  					u16 *brightness);
>  
> +/**
> + * mipi_dsi_generic_write - transmit data using a generic write packet
s/mipi_dsi_generic_write/mipi_dsi_generic_write_seq
(As the bot also reported)

with this fixed:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

> + * @dsi: DSI peripheral device
> + * @seq: buffer containing the payload
> + */
> +#define mipi_dsi_generic_write_seq(dsi, seq...) do {				\
> +		static const u8 d[] = { seq };					\
> +		struct device *dev = &dsi->dev;	\
> +		int ret;						\
> +		ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));	\
> +		if (ret < 0) {						\
> +			dev_err_ratelimited(dev, "transmit data failed: %d\n", ret); \
> +			return ret;						\
> +		}						\
> +	} while (0)
> +
(If you align '\' under each other it would be nicer, but I could see
that mipi_dsi_dcs_write_seq() do not do so).
>  /**
>   * mipi_dsi_dcs_write_seq - transmit a DCS command with payload
>   * @dsi: DSI peripheral device
> -- 
> 2.38.1
  

Hello Sam,

Thanks a lot for your feedback.

On 1/2/23 19:39, Sam Ravnborg wrote:
> Hi Javier.
> 
> On Wed, Dec 28, 2022 at 02:47:44AM +0100, Javier Martinez Canillas wrote:
>> Many panel drivers define dsi_dcs_write_seq() and dsi_generic_write_seq()
>> macros to send DCS commands and generic write packets respectively, with
>> the payload specified as a list of parameters instead of using arrays.
>>
>> There's already a macro for the former, introduced by commit 2a9e9daf75231
>> ("drm/mipi-dsi: Introduce mipi_dsi_dcs_write_seq macro") so drivers can be
>> changed to use that. But there isn't one yet for the latter, let's add it.
>>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>>
>>  include/drm/drm_mipi_dsi.h | 16 ++++++++++++++++
>>  1 file changed, 16 insertions(+)
>>
>> diff --git a/include/drm/drm_mipi_dsi.h b/include/drm/drm_mipi_dsi.h
>> index 20b21b577dea..c7c458131ba1 100644
>> --- a/include/drm/drm_mipi_dsi.h
>> +++ b/include/drm/drm_mipi_dsi.h
>> @@ -297,6 +297,22 @@ int mipi_dsi_dcs_set_display_brightness(struct mipi_dsi_device *dsi,
>>  int mipi_dsi_dcs_get_display_brightness(struct mipi_dsi_device *dsi,
>>  					u16 *brightness);
>>  
>> +/**
>> + * mipi_dsi_generic_write - transmit data using a generic write packet
> s/mipi_dsi_generic_write/mipi_dsi_generic_write_seq
> (As the bot also reported)
> 

Ups, sorry for missing that.

> with this fixed:
> Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
>

Thanks!
 
>> + * @dsi: DSI peripheral device
>> + * @seq: buffer containing the payload
>> + */
>> +#define mipi_dsi_generic_write_seq(dsi, seq...) do {				\
>> +		static const u8 d[] = { seq };					\
>> +		struct device *dev = &dsi->dev;	\
>> +		int ret;						\
>> +		ret = mipi_dsi_generic_write(dsi, d, ARRAY_SIZE(d));	\
>> +		if (ret < 0) {						\
>> +			dev_err_ratelimited(dev, "transmit data failed: %d\n", ret); \
>> +			return ret;						\
>> +		}						\
>> +	} while (0)
>> +
> (If you align '\' under each other it would be nicer, but I could see
> that mipi_dsi_dcs_write_seq() do not do so).

Yeah, I was actually thinking about doing like you suggested for this macro
but preferred to keep it consistent with the existing mipi_dsi_dcs_write_seq()
macro definition...

Maybe I can add a preparatory patch that just fixes the backslash characters
indent for mipi_dsi_dcs_write_seq() to be all aligned?
  

Hi Javier,

> > (If you align '\' under each other it would be nicer, but I could see
> > that mipi_dsi_dcs_write_seq() do not do so).
> 
> Yeah, I was actually thinking about doing like you suggested for this macro
> but preferred to keep it consistent with the existing mipi_dsi_dcs_write_seq()
> macro definition...
> 
> Maybe I can add a preparatory patch that just fixes the backslash characters
> indent for mipi_dsi_dcs_write_seq() to be all aligned?
Yep, that would be nice.

	Sam