| Message ID | 20221228014757.3170486-2-javierm@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | drm/panel: Make panel drivers use existing DSI write macros | expand |
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
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 + * @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) + /** * mipi_dsi_dcs_write_seq - transmit a DCS command with payload * @dsi: DSI peripheral device
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(+)