Hi Tomi,

On Thu, Jul 29, 2021 at 09:13:17AM +0300, Tomi Valkeinen wrote:

On 28/07/2021 18:37, Laurent Pinchart wrote:

...

On 64-bit platforms, the compiler complains that casting a void pointer to an unsigned int loses data. Cast the pointer to a uintptr_t unsigned to fix this.

Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com

drivers/gpu/drm/omapdrm/omap_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c index f86e20578143..c05d3975cb31 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.c +++ b/drivers/gpu/drm/omapdrm/omap_drv.c @@ -572,7 +572,7 @@ static int omapdrm_init(struct omap_drm_private *priv, struct device *dev) priv->dss->mgr_ops_priv = priv;

soc = soc_device_match(omapdrm_soc_devices);

• priv->omaprev = soc ? (unsigned int)soc->data : 0;

• priv->omaprev = soc ? (uintptr_t)soc->data : 0; priv->wq = alloc_ordered_workqueue("omapdrm", 0);

  mutex_init(&priv->list_lock);

Looks fine, although the subject sounds odd. Why was the cast "unsafe" before, and "safe" now?

The result is indeed exactly the same. It was safe before, because we know the value won't exceed 32 bits. I'll s/safely/without generating a warning/.

There's also another bunch of warnings I see:

I wonder how I missed those. Will fix.

drivers/gpu/drm/omapdrm/dss/dsi.c: In function ‘dsi_vc_send_long’: drivers/gpu/drm/omapdrm/dss/dsi.c:7:25: warning: format ‘%d’ expects argument of type ‘int’, but argument 3 has type ‘size_t’ {aka ‘const long unsigned int’} [-Wformat=] 7 | #define DSS_SUBSYS_NAME "DSI" | ^~~~~ drivers/gpu/drm/omapdrm/dss/dss.h:30:21: note: in expansion of macro ‘DSS_SUBSYS_NAME’ 30 | #define pr_fmt(fmt) DSS_SUBSYS_NAME ": " fmt | ^~~~~~~~~~~~~~~ ./include/linux/dynamic_debug.h:134:15: note: in expansion of macro ‘pr_fmt’ 134 | func(&id, ##__VA_ARGS__); \ | ^~~~~~~~~~~ ./include/linux/dynamic_debug.h:152:2: note: in expansion of macro ‘__dynamic_func_call’ 152 | __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~ ./include/linux/dynamic_debug.h:162:2: note: in expansion of macro ‘_dynamic_func_call’ 162 | _dynamic_func_call(fmt, __dynamic_pr_debug, \ | ^~~~~~~~~~~~~~~~~~ ./include/linux/printk.h:471:2: note: in expansion of macro ‘dynamic_pr_debug’ 471 | dynamic_pr_debug(fmt, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~ drivers/gpu/drm/omapdrm/dss/dss.h:36:2: note: in expansion of macro ‘pr_debug’ 36 | pr_debug(format, ## __VA_ARGS__) | ^~~~~~~~ drivers/gpu/drm/omapdrm/dss/dsi.c:2097:3: note: in expansion of macro ‘DSSDBG’ 2097 | DSSDBG("dsi_vc_send_long, %d bytes\n", msg->tx_len); | ^~~~~~ In file included from ./include/linux/printk.h:7, from ./include/linux/kernel.h:19, from drivers/gpu/drm/omapdrm/dss/dsi.c:9: drivers/gpu/drm/omapdrm/dss/dsi.c: In function ‘dsi_vc_generic_read’: ./include/linux/kern_levels.h:5:18: warning: format ‘%d’ expects argument of type ‘int’, but argument 4 has type ‘size_t’ {aka ‘const long unsigned int’} [-Wformat=] 5 | #define KERN_SOH "\001" /* ASCII Start Of Header */ | ^~~~~~ ./include/linux/kern_levels.h:11:18: note: in expansion of macro ‘KERN_SOH’ 11 | #define KERN_ERR KERN_SOH "3" /* error conditions */ | ^~~~~~~~ ./include/linux/printk.h:390:9: note: in expansion of macro ‘KERN_ERR’ 390 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__) | ^~~~~~~~ drivers/gpu/drm/omapdrm/dss/dss.h:40:2: note: in expansion of macro ‘pr_err’ 40 | pr_err("omapdss " DSS_SUBSYS_NAME " error: " format, ##__VA_ARGS__) | ^~~~~~ drivers/gpu/drm/omapdrm/dss/dsi.c:2393:2: note: in expansion of macro ‘DSSERR’ 2393 | DSSERR("%s(vc %d, reqlen %d) failed\n", __func__, vc, msg->tx_len); | ^~~~~~

On 7/28/21 5:37 PM, Laurent Pinchart wrote:

The correct format specifier for size_t is %zu. Using %d (or %u) generates a warning on 64-bit platforms. Fix it.

Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com

drivers/gpu/drm/sti/sti_hqvdp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hqvdp.c b/drivers/gpu/drm/sti/sti_hqvdp.c index d09b08995b12..3c61ba8b43e0 100644 --- a/drivers/gpu/drm/sti/sti_hqvdp.c +++ b/drivers/gpu/drm/sti/sti_hqvdp.c @@ -927,12 +927,12 @@ static void sti_hqvdp_start_xp70(struct sti_hqvdp *hqvdp)

header = (struct fw_header *)firmware->data; if (firmware->size < sizeof(*header)) {

• DRM_ERROR("Invalid firmware size (%d)\n", firmware->size);
  

• DRM_ERROR("Invalid firmware size (%zu)\n", firmware->size);
  

  goto out; } if ((sizeof(*header) + header->rd_size + header->wr_size + header->pmem_size + header->dmem_size) != firmware->size) {

• DRM_ERROR("Invalid fmw structure (%d+%d+%d+%d+%d != %d)\n",
  

• DRM_ERROR("Invalid fmw structure (%zu+%d+%d+%d+%d != %zu)\n",
  

Hi Laurent,

Looping Benjamin (new email address) and Alain (future maintainer of drm/sti).

Reviewed-by: Philippe Cornu philippe.cornu@foss.st.com Many thanks for your patch, Philippe :-)

	  sizeof(*header), header->rd_size, header->wr_size,
	  header->pmem_size, header->dmem_size,
	  firmware->size);

Hi Laurent,

On Wed, Jul 28, 2021 at 5:37 PM Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com wrote:

To extend test coverage, relax the dependency on ARCH_MXC to also enable compilation when COMPILE_TEST is selected.

Signed-off-by: Laurent Pinchart laurent.pinchart+renesas@ideasonboard.com

Thanks for your patch!

--- a/drivers/gpu/drm/imx/dcss/Kconfig +++ b/drivers/gpu/drm/imx/dcss/Kconfig @@ -3,7 +3,8 @@ config DRM_IMX_DCSS select IMX_IRQSTEER select DRM_KMS_CMA_HELPER select VIDEOMODE_HELPERS

•   depends on DRM && ARCH_MXC && ARM64
  

•   depends on DRM
  

•   depends on ARM64 && (ARCH_MXC || COMPILE_TEST)
  

As you now have two depends statements, I think this would be easier to read by maintaining a strict separation between "hard" and "soft" dependencies:

depends on DRM && ARM64 depends on ARCH_MXC || COMPILE_TEST

Gr{oetje,eeting}s,

Geert