[PATCH 0/4] drm: core: Convert logging to drm_* functions.

[PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Suraj Upadhyay]

[-- Attachment #1: Type: text/plain, Size: 783 bytes --]


This patchset converts logging to drm_* functions in drm core.

The following functions have been converted to their respective
DRM alternatives :
dev_info()      --> drm_info()
dev_err()       --> drm_err()
dev_warn()      --> drm_warn()
dev_err_once()  --> drm_err_once().

Suraj Upadhyay (4):
  drm: mipi-dsi: Convert logging to drm_* functions.
  drm: mipi-dbi: Convert logging to drm_* functions.
  drm: edid: Convert logging to drm_* functions.
  drm: fb-helper: Convert logging to drm_* functions.

 drivers/gpu/drm/drm_edid.c      |  7 +++----
 drivers/gpu/drm/drm_fb_helper.c |  2 +-
 drivers/gpu/drm/drm_mipi_dbi.c  |  4 ++--
 drivers/gpu/drm/drm_mipi_dsi.c  | 15 +++++++--------
 4 files changed, 13 insertions(+), 15 deletions(-)

-- 
2.17.1



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 1/4] drm: mipi-dsi: Convert logging to drm_* functions.

[Suraj Upadhyay]

Convert logging errors from dev_err() to drm_err().

Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 07102d8da58f..5dd475e82995 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 
 #include <drm/drm_dsc.h>
+#include <drm/drm_print.h>
 #include <video/mipi_display.h>
 
 /**
@@ -155,19 +156,18 @@ static int mipi_dsi_device_add(struct mipi_dsi_device *dsi)
 static struct mipi_dsi_device *
 of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
 {
-	struct device *dev = host->dev;
 	struct mipi_dsi_device_info info = { };
 	int ret;
 	u32 reg;
 
 	if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
-		dev_err(dev, "modalias failure on %pOF\n", node);
+		drm_err(host, "modalias failure on %pOF\n", node);
 		return ERR_PTR(-EINVAL);
 	}
 
 	ret = of_property_read_u32(node, "reg", &reg);
 	if (ret) {
-		dev_err(dev, "device node %pOF has no valid reg property: %d\n",
+		drm_err(host, "device node %pOF has no valid reg property: %d\n",
 			node, ret);
 		return ERR_PTR(-EINVAL);
 	}
@@ -202,22 +202,21 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
 			      const struct mipi_dsi_device_info *info)
 {
 	struct mipi_dsi_device *dsi;
-	struct device *dev = host->dev;
 	int ret;
 
 	if (!info) {
-		dev_err(dev, "invalid mipi_dsi_device_info pointer\n");
+		drm_err(host, "invalid mipi_dsi_device_info pointer\n");
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (info->channel > 3) {
-		dev_err(dev, "invalid virtual channel: %u\n", info->channel);
+		drm_err(host, "invalid virtual channel: %u\n", info->channel);
 		return ERR_PTR(-EINVAL);
 	}
 
 	dsi = mipi_dsi_device_alloc(host);
 	if (IS_ERR(dsi)) {
-		dev_err(dev, "failed to allocate DSI device %ld\n",
+		drm_err(host, "failed to allocate DSI device %ld\n",
 			PTR_ERR(dsi));
 		return dsi;
 	}
@@ -228,7 +227,7 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
 
 	ret = mipi_dsi_device_add(dsi);
 	if (ret) {
-		dev_err(dev, "failed to add DSI device %d\n", ret);
+		drm_err(host, "failed to add DSI device %d\n", ret);
 		kfree(dsi);
 		return ERR_PTR(ret);
 	}
-- 
2.17.1

[PATCH 1/4] drm: mipi-dsi: Convert logging to drm_* functions.

[Suraj Upadhyay]

[-- Attachment #1: Type: text/plain, Size: 2442 bytes --]

Convert logging errors from dev_err() to drm_err().

Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
---
 drivers/gpu/drm/drm_mipi_dsi.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
index 07102d8da58f..5dd475e82995 100644
--- a/drivers/gpu/drm/drm_mipi_dsi.c
+++ b/drivers/gpu/drm/drm_mipi_dsi.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 
 #include <drm/drm_dsc.h>
+#include <drm/drm_print.h>
 #include <video/mipi_display.h>
 
 /**
@@ -155,19 +156,18 @@ static int mipi_dsi_device_add(struct mipi_dsi_device *dsi)
 static struct mipi_dsi_device *
 of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
 {
-	struct device *dev = host->dev;
 	struct mipi_dsi_device_info info = { };
 	int ret;
 	u32 reg;
 
 	if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
-		dev_err(dev, "modalias failure on %pOF\n", node);
+		drm_err(host, "modalias failure on %pOF\n", node);
 		return ERR_PTR(-EINVAL);
 	}
 
 	ret = of_property_read_u32(node, "reg", &reg);
 	if (ret) {
-		dev_err(dev, "device node %pOF has no valid reg property: %d\n",
+		drm_err(host, "device node %pOF has no valid reg property: %d\n",
 			node, ret);
 		return ERR_PTR(-EINVAL);
 	}
@@ -202,22 +202,21 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
 			      const struct mipi_dsi_device_info *info)
 {
 	struct mipi_dsi_device *dsi;
-	struct device *dev = host->dev;
 	int ret;
 
 	if (!info) {
-		dev_err(dev, "invalid mipi_dsi_device_info pointer\n");
+		drm_err(host, "invalid mipi_dsi_device_info pointer\n");
 		return ERR_PTR(-EINVAL);
 	}
 
 	if (info->channel > 3) {
-		dev_err(dev, "invalid virtual channel: %u\n", info->channel);
+		drm_err(host, "invalid virtual channel: %u\n", info->channel);
 		return ERR_PTR(-EINVAL);
 	}
 
 	dsi = mipi_dsi_device_alloc(host);
 	if (IS_ERR(dsi)) {
-		dev_err(dev, "failed to allocate DSI device %ld\n",
+		drm_err(host, "failed to allocate DSI device %ld\n",
 			PTR_ERR(dsi));
 		return dsi;
 	}
@@ -228,7 +227,7 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
 
 	ret = mipi_dsi_device_add(dsi);
 	if (ret) {
-		dev_err(dev, "failed to add DSI device %d\n", ret);
+		drm_err(host, "failed to add DSI device %d\n", ret);
 		kfree(dsi);
 		return ERR_PTR(ret);
 	}
-- 
2.17.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/4] drm: mipi-dsi: Convert logging to drm_* functions.

[Sam Ravnborg]

On Tue, Jul 07, 2020 at 09:58:48PM +0530, Suraj Upadhyay wrote:
> Convert logging errors from dev_err() to drm_err().
> 
> Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>

Thanks. Applied to drm-misc-next as there was no more logging conversion
to do in this file.

	Sam

> ---
>  drivers/gpu/drm/drm_mipi_dsi.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index 07102d8da58f..5dd475e82995 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -34,6 +34,7 @@
>  #include <linux/slab.h>
>  
>  #include <drm/drm_dsc.h>
> +#include <drm/drm_print.h>
>  #include <video/mipi_display.h>
>  
>  /**
> @@ -155,19 +156,18 @@ static int mipi_dsi_device_add(struct mipi_dsi_device *dsi)
>  static struct mipi_dsi_device *
>  of_mipi_dsi_device_add(struct mipi_dsi_host *host, struct device_node *node)
>  {
> -	struct device *dev = host->dev;
>  	struct mipi_dsi_device_info info = { };
>  	int ret;
>  	u32 reg;
>  
>  	if (of_modalias_node(node, info.type, sizeof(info.type)) < 0) {
> -		dev_err(dev, "modalias failure on %pOF\n", node);
> +		drm_err(host, "modalias failure on %pOF\n", node);
>  		return ERR_PTR(-EINVAL);
>  	}
>  
>  	ret = of_property_read_u32(node, "reg", &reg);
>  	if (ret) {
> -		dev_err(dev, "device node %pOF has no valid reg property: %d\n",
> +		drm_err(host, "device node %pOF has no valid reg property: %d\n",
>  			node, ret);
>  		return ERR_PTR(-EINVAL);
>  	}
> @@ -202,22 +202,21 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
>  			      const struct mipi_dsi_device_info *info)
>  {
>  	struct mipi_dsi_device *dsi;
> -	struct device *dev = host->dev;
>  	int ret;
>  
>  	if (!info) {
> -		dev_err(dev, "invalid mipi_dsi_device_info pointer\n");
> +		drm_err(host, "invalid mipi_dsi_device_info pointer\n");
>  		return ERR_PTR(-EINVAL);
>  	}
>  
>  	if (info->channel > 3) {
> -		dev_err(dev, "invalid virtual channel: %u\n", info->channel);
> +		drm_err(host, "invalid virtual channel: %u\n", info->channel);
>  		return ERR_PTR(-EINVAL);
>  	}
>  
>  	dsi = mipi_dsi_device_alloc(host);
>  	if (IS_ERR(dsi)) {
> -		dev_err(dev, "failed to allocate DSI device %ld\n",
> +		drm_err(host, "failed to allocate DSI device %ld\n",
>  			PTR_ERR(dsi));
>  		return dsi;
>  	}
> @@ -228,7 +227,7 @@ mipi_dsi_device_register_full(struct mipi_dsi_host *host,
>  
>  	ret = mipi_dsi_device_add(dsi);
>  	if (ret) {
> -		dev_err(dev, "failed to add DSI device %d\n", ret);
> +		drm_err(host, "failed to add DSI device %d\n", ret);
>  		kfree(dsi);
>  		return ERR_PTR(ret);
>  	}
> -- 
> 2.17.1
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/4] drm: mipi-dbi: Convert logging to drm_* functions.

[Suraj Upadhyay]

[-- Attachment #1: Type: text/plain, Size: 1133 bytes --]

Convert logging of errors once from dev_err_once() to drm_err_once().

Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
---
 drivers/gpu/drm/drm_mipi_dbi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_mipi_dbi.c b/drivers/gpu/drm/drm_mipi_dbi.c
index 79532b9a324a..ccfb6eb1a29f 100644
--- a/drivers/gpu/drm/drm_mipi_dbi.c
+++ b/drivers/gpu/drm/drm_mipi_dbi.c
@@ -225,7 +225,7 @@ int mipi_dbi_buf_copy(void *dst, struct drm_framebuffer *fb,
 		drm_fb_xrgb8888_to_rgb565(dst, src, fb, clip, swap);
 		break;
 	default:
-		dev_err_once(fb->dev->dev, "Format is not supported: %s\n",
+		drm_err_once(fb->dev, "Format is not supported: %s\n",
 			     drm_get_format_name(fb->format->format,
 						 &format_name));
 		return -EINVAL;
@@ -295,7 +295,7 @@ static void mipi_dbi_fb_dirty(struct drm_framebuffer *fb, struct drm_rect *rect)
 				   width * height * 2);
 err_msg:
 	if (ret)
-		dev_err_once(fb->dev->dev, "Failed to update display %d\n", ret);
+		drm_err_once(fb->dev, "Failed to update display %d\n", ret);
 
 	drm_dev_exit(idx);
 }
-- 
2.17.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH 3/4] drm: edid: Convert logging to drm_* functions.

[Suraj Upadhyay]

[-- Attachment #1: Type: text/plain, Size: 1156 bytes --]

Change logging of warnings to drm_warn() form dev_warn().

Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
---
 drivers/gpu/drm/drm_edid.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 31496b6cfc56..ad7a1f9817ed 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1844,9 +1844,8 @@ static void connector_bad_edid(struct drm_connector *connector,
 	if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
 		return;
 
-	dev_warn(connector->dev->dev,
-		 "%s: EDID is invalid:\n",
-		 connector->name);
+	drm_warn(connector->dev, "%s: EDID is invalid:\n", connector->name);
+
 	for (i = 0; i < num_blocks; i++) {
 		u8 *block = edid + i * EDID_LENGTH;
 		char prefix[20];
@@ -5284,7 +5283,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
 	}
 	if (!drm_edid_is_valid(edid)) {
 		clear_eld(connector);
-		dev_warn(connector->dev->dev, "%s: EDID invalid.\n",
+		drm_warn(connector->dev, "%s: EDID invalid.\n",
 			 connector->name);
 		return 0;
 	}
-- 
2.17.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 3/4] drm: edid: Convert logging to drm_* functions.

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1519 bytes --]

Thanks!

Am 07.07.20 um 18:36 schrieb Suraj Upadhyay:
> Change logging of warnings to drm_warn() form dev_warn().
> 
> Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>  drivers/gpu/drm/drm_edid.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 31496b6cfc56..ad7a1f9817ed 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1844,9 +1844,8 @@ static void connector_bad_edid(struct drm_connector *connector,
>  	if (connector->bad_edid_counter++ && !drm_debug_enabled(DRM_UT_KMS))
>  		return;
>  
> -	dev_warn(connector->dev->dev,
> -		 "%s: EDID is invalid:\n",
> -		 connector->name);
> +	drm_warn(connector->dev, "%s: EDID is invalid:\n", connector->name);
> +
>  	for (i = 0; i < num_blocks; i++) {
>  		u8 *block = edid + i * EDID_LENGTH;
>  		char prefix[20];
> @@ -5284,7 +5283,7 @@ int drm_add_edid_modes(struct drm_connector *connector, struct edid *edid)
>  	}
>  	if (!drm_edid_is_valid(edid)) {
>  		clear_eld(connector);
> -		dev_warn(connector->dev->dev, "%s: EDID invalid.\n",
> +		drm_warn(connector->dev, "%s: EDID invalid.\n",
>  			 connector->name);
>  		return 0;
>  	}
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

[PATCH 4/4] drm: fb-helper: Convert logging to drm_* functions.

[Suraj Upadhyay]

[-- Attachment #1: Type: text/plain, Size: 737 bytes --]

Change logging information from dev_info() to drm_info().

Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>
---
 drivers/gpu/drm/drm_fb_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 5609e164805f..88146f7245c5 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -1819,7 +1819,7 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
 	if (ret < 0)
 		return ret;
 
-	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
+	drm_info(dev, "fb%d: %s frame buffer device\n",
 		 info->node, info->fix.id);
 
 	mutex_lock(&kernel_fb_helper_lock);
-- 
2.17.1


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 4/4] drm: fb-helper: Convert logging to drm_* functions.

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1069 bytes --]



Am 07.07.20 um 18:37 schrieb Suraj Upadhyay:
> Change logging information from dev_info() to drm_info().
> 
> Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 5609e164805f..88146f7245c5 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1819,7 +1819,7 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
>  	if (ret < 0)
>  		return ret;
>  
> -	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
> +	drm_info(dev, "fb%d: %s frame buffer device\n",
>  		 info->node, info->fix.id);
>  
>  	mutex_lock(&kernel_fb_helper_lock);
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 516 bytes --]

Re: [PATCH 4/4] drm: fb-helper: Convert logging to drm_* functions.

[Sam Ravnborg]

On Tue, Jul 07, 2020 at 10:07:21PM +0530, Suraj Upadhyay wrote:
> Change logging information from dev_info() to drm_info().
> 
> Signed-off-by: Suraj Upadhyay <usuraj35@gmail.com>

Thanks. Applied to drm-misc-next.

Also, because there was no more logging functions that needed an update.

	Sam

> ---
>  drivers/gpu/drm/drm_fb_helper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 5609e164805f..88146f7245c5 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1819,7 +1819,7 @@ __drm_fb_helper_initial_config_and_unlock(struct drm_fb_helper *fb_helper,
>  	if (ret < 0)
>  		return ret;
>  
> -	dev_info(dev->dev, "fb%d: %s frame buffer device\n",
> +	drm_info(dev, "fb%d: %s frame buffer device\n",
>  		 info->node, info->fix.id);
>  
>  	mutex_lock(&kernel_fb_helper_lock);
> -- 
> 2.17.1
> 



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

Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Sam Ravnborg]

Hi Suraj.

On Tue, Jul 07, 2020 at 10:04:14PM +0530, Suraj Upadhyay wrote:
> 
> This patchset converts logging to drm_* functions in drm core.
> 
> The following functions have been converted to their respective
> DRM alternatives :
> dev_info()      --> drm_info()
> dev_err()       --> drm_err()
> dev_warn()      --> drm_warn()
> dev_err_once()  --> drm_err_once().

I would prefer that DRM_* logging in the same files are converted in the
same patch. So we have one logging conversion patch for each file you
touches and that we do not need to re-vist the files later to change
another set of logging functions.
If possible WARN_* should also be converted to drm_WARN_*
If patch is too large, then split them up but again lets have all
logging updated when we touch a file.

Care to take a look at this approach?

Also please consider if coccinelle can make this job easier.
There is a lot of files...

	Sam

> 
> Suraj Upadhyay (4):
>   drm: mipi-dsi: Convert logging to drm_* functions.
>   drm: mipi-dbi: Convert logging to drm_* functions.
>   drm: edid: Convert logging to drm_* functions.
>   drm: fb-helper: Convert logging to drm_* functions.
> 
>  drivers/gpu/drm/drm_edid.c      |  7 +++----
>  drivers/gpu/drm/drm_fb_helper.c |  2 +-
>  drivers/gpu/drm/drm_mipi_dbi.c  |  4 ++--
>  drivers/gpu/drm/drm_mipi_dsi.c  | 15 +++++++--------
>  4 files changed, 13 insertions(+), 15 deletions(-)
> 
> -- 
> 2.17.1
> 
> 



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

Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Suraj Upadhyay]

[-- Attachment #1: Type: text/plain, Size: 2734 bytes --]

On Fri, Jul 10, 2020 at 07:56:43PM +0200, Sam Ravnborg wrote:
> Hi Suraj.
> 
> On Tue, Jul 07, 2020 at 10:04:14PM +0530, Suraj Upadhyay wrote:
> > 
> > This patchset converts logging to drm_* functions in drm core.
> > 
> > The following functions have been converted to their respective
> > DRM alternatives :
> > dev_info()      --> drm_info()
> > dev_err()       --> drm_err()
> > dev_warn()      --> drm_warn()
> > dev_err_once()  --> drm_err_once().
> 
> I would prefer that DRM_* logging in the same files are converted in the
> same patch. So we have one logging conversion patch for each file you
> touches and that we do not need to re-vist the files later to change
> another set of logging functions.

Agreed.

> If possible WARN_* should also be converted to drm_WARN_*
> If patch is too large, then split them up but again lets have all
> logging updated when we touch a file.
> 
> Care to take a look at this approach?
>

Hii,
	The problem with WARN_* macros is that they are used without any
drm device context. For example [this use here](https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_edid.c#n1667) in drm_edid.c,
doesn't have a drm device context and only has one argument (namely !raw_edid).
There are many more such use cases.

And also there were cases where dev_* logging functions didn't have any
drm_device context.

I would be very glad, if we came up with a possible solution to this
problem. I think we should develop drm_* logging APIs which could print
contextless logs (which would possibly be midlyering) or give every situation a context.

> Also please consider if coccinelle can make this job easier.
> There is a lot of files...

I totally agree with you. I will remember this next time.

But here, in this patchset I have tried to convert all possible
cases of conversion, i.e. I have changed logging wherever there was a
drm_device context.

Thanks.

> 	Sam
> 
> > 
> > Suraj Upadhyay (4):
> >   drm: mipi-dsi: Convert logging to drm_* functions.
> >   drm: mipi-dbi: Convert logging to drm_* functions.
> >   drm: edid: Convert logging to drm_* functions.
> >   drm: fb-helper: Convert logging to drm_* functions.
> > 
> >  drivers/gpu/drm/drm_edid.c      |  7 +++----
> >  drivers/gpu/drm/drm_fb_helper.c |  2 +-
> >  drivers/gpu/drm/drm_mipi_dbi.c  |  4 ++--
> >  drivers/gpu/drm/drm_mipi_dsi.c  | 15 +++++++--------
> >  4 files changed, 13 insertions(+), 15 deletions(-)
> > 
> > -- 
> > 2.17.1
> > 
> > 
> 
> 
> 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Joe Perches]

On Sat, 2020-07-11 at 20:41 +0530, Suraj Upadhyay wrote:
> On Fri, Jul 10, 2020 at 07:56:43PM +0200, Sam Ravnborg wrote:
> > Hi Suraj.
> > 
> > On Tue, Jul 07, 2020 at 10:04:14PM +0530, Suraj Upadhyay wrote:
> > > This patchset converts logging to drm_* functions in drm core.
> > > 
> > > The following functions have been converted to their respective
> > > DRM alternatives :
> > > dev_info()      --> drm_info()
> > > dev_err()       --> drm_err()
> > > dev_warn()      --> drm_warn()
> > > dev_err_once()  --> drm_err_once().
> > 
> > I would prefer that DRM_* logging in the same files are converted in the
> > same patch. So we have one logging conversion patch for each file you
> > touches and that we do not need to re-vist the files later to change
> > another set of logging functions.
> 
> Agreed.
> 
> > If possible WARN_* should also be converted to drm_WARN_*
> > If patch is too large, then split them up but again lets have all
> > logging updated when we touch a file.
> > 
> > Care to take a look at this approach?
> > 
> 
> Hii,
> 	The problem with WARN_* macros is that they are used without any
> drm device context. For example [this use here](https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_edid.c#n1667) in drm_edid.c,
> doesn't have a drm device context and only has one argument (namely !raw_edid).
> There are many more such use cases.
> 
> And also there were cases where dev_* logging functions didn't have any
> drm_device context.

Perhaps change the __drm_printk macro to not
dereference the drm argument when NULL.

A trivial but perhaps inefficient way might be
used like:

	drm_<level>(NULL, fmt, ...)

---
 include/drm/drm_print.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 1c9417430d08..9323a8f46b3c 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -395,8 +395,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
 
 /* Helper for struct drm_device based logging. */
 #define __drm_printk(drm, level, type, fmt, ...)			\
-	dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
-
+	dev_##level##type((drm) ? (drm)->dev : NULL, "[drm] " fmt,	\
+			  ##__VA_ARGS__)
 
 #define drm_info(drm, fmt, ...)					\
 	__drm_printk((drm), info,, fmt, ##__VA_ARGS__)

Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Suraj Upadhyay]

[-- Attachment #1: Type: text/plain, Size: 3239 bytes --]

On Sat, Jul 11, 2020 at 11:16:33AM -0700, Joe Perches wrote:
> On Sat, 2020-07-11 at 20:41 +0530, Suraj Upadhyay wrote:
> > On Fri, Jul 10, 2020 at 07:56:43PM +0200, Sam Ravnborg wrote:
> > > Hi Suraj.
> > > 
> > > On Tue, Jul 07, 2020 at 10:04:14PM +0530, Suraj Upadhyay wrote:
> > > > This patchset converts logging to drm_* functions in drm core.
> > > > 
> > > > The following functions have been converted to their respective
> > > > DRM alternatives :
> > > > dev_info()      --> drm_info()
> > > > dev_err()       --> drm_err()
> > > > dev_warn()      --> drm_warn()
> > > > dev_err_once()  --> drm_err_once().
> > > 
> > > I would prefer that DRM_* logging in the same files are converted in the
> > > same patch. So we have one logging conversion patch for each file you
> > > touches and that we do not need to re-vist the files later to change
> > > another set of logging functions.
> > 
> > Agreed.
> > 
> > > If possible WARN_* should also be converted to drm_WARN_*
> > > If patch is too large, then split them up but again lets have all
> > > logging updated when we touch a file.
> > > 
> > > Care to take a look at this approach?
> > > 
> > 
> > Hii,
> > 	The problem with WARN_* macros is that they are used without any
> > drm device context. For example [this use here](https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_edid.c#n1667) in drm_edid.c,
> > doesn't have a drm device context and only has one argument (namely !raw_edid).
> > There are many more such use cases.
> > 
> > And also there were cases where dev_* logging functions didn't have any
> > drm_device context.
> 
> Perhaps change the __drm_printk macro to not
> dereference the drm argument when NULL.
> 
> A trivial but perhaps inefficient way might be
> used like:
> 
> 	drm_<level>(NULL, fmt, ...)
> 
> ---
>  include/drm/drm_print.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 1c9417430d08..9323a8f46b3c 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -395,8 +395,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
>  
>  /* Helper for struct drm_device based logging. */
>  #define __drm_printk(drm, level, type, fmt, ...)			\
> -	dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
> -
> +	dev_##level##type((drm) ? (drm)->dev : NULL, "[drm] " fmt,	\
> +			  ##__VA_ARGS__)
>  
>  #define drm_info(drm, fmt, ...)					\
>  	__drm_printk((drm), info,, fmt, ##__VA_ARGS__)
> 

Hi Joe,
	Thanks for your input.
But I don't think that this change would be a good idea as we are
supposed to find or make a substitute of WARN_* macros which
take a `condition` as an argument and check for its truth.
And I guess passing a NULL to dev_<level> would cause a format warning.

Also, the WARN_* macros are doing their job fine, and passing a NULL
value everytime you want to warn about a certain condition at a
particular line, doesn't seem good to me.

Thus, I think that WARN_* macros should be untouched.

I would like to hear what the MAINTAINERS think.

Thanks and Cheers,

Suraj Upadhyay.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Joe Perches]

On Mon, 2020-07-13 at 00:24 +0530, Suraj Upadhyay wrote:
> On Sat, Jul 11, 2020 at 11:16:33AM -0700, Joe Perches wrote:
[]
> > Perhaps change the __drm_printk macro to not
> > dereference the drm argument when NULL.
> > 
> > A trivial but perhaps inefficient way might be
> > used like:
> > 
> > 	drm_<level>(NULL, fmt, ...)
[]
> > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
[]
> > @@ -395,8 +395,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> >  
> >  /* Helper for struct drm_device based logging. */
> >  #define __drm_printk(drm, level, type, fmt, ...)			\
> > -	dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
> > -
> > +	dev_##level##type((drm) ? (drm)->dev : NULL, "[drm] " fmt,	\
> > +			  ##__VA_ARGS__)
> >  
> >  #define drm_info(drm, fmt, ...)					\
> >  	__drm_printk((drm), info,, fmt, ##__VA_ARGS__)
> > 
> 
> Hi Joe,
> 	Thanks for your input.
> But I don't think that this change would be a good idea as we are
> supposed to find or make a substitute of WARN_* macros which
> take a `condition` as an argument and check for its truth.
> And I guess passing a NULL to dev_<level> would cause a format warning.
> 
> Also, the WARN_* macros are doing their job fine, and passing a NULL
> value everytime you want to warn about a certain condition at a
> particular line, doesn't seem good to me.
> 
> Thus, I think that WARN_* macros should be untouched.

So do I but the suggestion was not about WARN macros
only about drm_<level> macros and possibly unnecessary
conversions to dev_<level> when a drm_device context
is unavailable.

Also, you don't have to guess, the code is there for
you to inspect.

dev_<level> when a NULL is used as the first argument
emits "(NULL device *)" instead of dev_driver_string(dev)
and dev_name(dev).

See: drivers/base/core.c::__dev_printk()

Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Suraj Upadhyay]

[-- Attachment #1: Type: text/plain, Size: 2597 bytes --]

On Sun, Jul 12, 2020 at 12:07:45PM -0700, Joe Perches wrote:
> On Mon, 2020-07-13 at 00:24 +0530, Suraj Upadhyay wrote:
> > On Sat, Jul 11, 2020 at 11:16:33AM -0700, Joe Perches wrote:
> []
> > > Perhaps change the __drm_printk macro to not
> > > dereference the drm argument when NULL.
> > > 
> > > A trivial but perhaps inefficient way might be
> > > used like:
> > > 
> > > 	drm_<level>(NULL, fmt, ...)
> []
> > > diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> []
> > > @@ -395,8 +395,8 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > >  
> > >  /* Helper for struct drm_device based logging. */
> > >  #define __drm_printk(drm, level, type, fmt, ...)			\
> > > -	dev_##level##type((drm)->dev, "[drm] " fmt, ##__VA_ARGS__)
> > > -
> > > +	dev_##level##type((drm) ? (drm)->dev : NULL, "[drm] " fmt,	\
> > > +			  ##__VA_ARGS__)
> > >  
> > >  #define drm_info(drm, fmt, ...)					\
> > >  	__drm_printk((drm), info,, fmt, ##__VA_ARGS__)
> > > 
> > 
> > Hi Joe,
> > 	Thanks for your input.
> > But I don't think that this change would be a good idea as we are
> > supposed to find or make a substitute of WARN_* macros which
> > take a `condition` as an argument and check for its truth.
> > And I guess passing a NULL to dev_<level> would cause a format warning.
> > 
> > Also, the WARN_* macros are doing their job fine, and passing a NULL
> > value everytime you want to warn about a certain condition at a
> > particular line, doesn't seem good to me.
> > 
> > Thus, I think that WARN_* macros should be untouched.
> 
> So do I but the suggestion was not about WARN macros
> only about drm_<level> macros and possibly unnecessary
> conversions to dev_<level> when a drm_device context
> is unavailable.
> 
> Also, you don't have to guess, the code is there for
> you to inspect.
> 
> dev_<level> when a NULL is used as the first argument
> emits "(NULL device *)" instead of dev_driver_string(dev)
> and dev_name(dev).
> 
> See: drivers/base/core.c::__dev_printk()

Yes, Thanks my bad.
But the dev_<level> usages in drm/* always have a context and doesn't need
NULL to be passed, i.e. some of them have only a `struct device` context which
cannot be simply converted into drm_<level> since they take a struct
pointer with a `dev` member and not a `dev` itself.
What we can convert is calls to dev_<level> with a drm_device context or
with a struct pointer which has a dev member.

And, I really want the MAINTAINERS to look into it.

Thanks and Cheers,

Suraj Upadhyay.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

[Sam Ravnborg]

Hi Suraj.

On Sat, Jul 11, 2020 at 08:41:26PM +0530, Suraj Upadhyay wrote:
> On Fri, Jul 10, 2020 at 07:56:43PM +0200, Sam Ravnborg wrote:
> > Hi Suraj.
> > 
> > On Tue, Jul 07, 2020 at 10:04:14PM +0530, Suraj Upadhyay wrote:
> > > 
> > > This patchset converts logging to drm_* functions in drm core.
> > > 
> > > The following functions have been converted to their respective
> > > DRM alternatives :
> > > dev_info()      --> drm_info()
> > > dev_err()       --> drm_err()
> > > dev_warn()      --> drm_warn()
> > > dev_err_once()  --> drm_err_once().
> > 
> > I would prefer that DRM_* logging in the same files are converted in the
> > same patch. So we have one logging conversion patch for each file you
> > touches and that we do not need to re-vist the files later to change
> > another set of logging functions.
> 
> Agreed.
> 
> > If possible WARN_* should also be converted to drm_WARN_*
> > If patch is too large, then split them up but again lets have all
> > logging updated when we touch a file.
> > 
> > Care to take a look at this approach?
> >
> 
> Hii,
> 	The problem with WARN_* macros is that they are used without any
> drm device context. For example [this use here](https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/drm_edid.c#n1667) in drm_edid.c,
> doesn't have a drm device context and only has one argument (namely !raw_edid).
> There are many more such use cases.
> 
> And also there were cases where dev_* logging functions didn't have any
> drm_device context.
> 
> I would be very glad, if we came up with a possible solution to this
> problem. I think we should develop drm_* logging APIs which could print
> contextless logs (which would possibly be midlyering) or give every situation a context.

This was discussed in the past.
For now the focus is migrating existing logging to the new drm_ based
logging. Preferably using coccinelle.
when we are more or less covered there we can discuss if we will add
more logging functions.

> 
> > Also please consider if coccinelle can make this job easier.
> > There is a lot of files...
> 
> I totally agree with you. I will remember this next time.
> 
> But here, in this patchset I have tried to convert all possible
> cases of conversion, i.e. I have changed logging wherever there was a
> drm_device context.

Please note this in the changelog when you resend.
I delete your original patches so they are gone now from my mail-box.
Yeah, could dig them up somewhere, but it is easier to let you resend
them and then we can have an updated changelog too.

	Sam

> 
> Thanks.
> 
> > 	Sam
> > 
> > > 
> > > Suraj Upadhyay (4):
> > >   drm: mipi-dsi: Convert logging to drm_* functions.
> > >   drm: mipi-dbi: Convert logging to drm_* functions.
> > >   drm: edid: Convert logging to drm_* functions.
> > >   drm: fb-helper: Convert logging to drm_* functions.
> > > 
> > >  drivers/gpu/drm/drm_edid.c      |  7 +++----
> > >  drivers/gpu/drm/drm_fb_helper.c |  2 +-
> > >  drivers/gpu/drm/drm_mipi_dbi.c  |  4 ++--
> > >  drivers/gpu/drm/drm_mipi_dsi.c  | 15 +++++++--------
> > >  4 files changed, 13 insertions(+), 15 deletions(-)
> > > 
> > > -- 
> > > 2.17.1
> > > 
> > > 
> > 
> > 
> > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 



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