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
Convert logging of errors once from dev_err_once() to drm_err_once().
Signed-off-by: Suraj Upadhyay <[email protected]>
---
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
Change logging information from dev_info() to drm_info().
Signed-off-by: Suraj Upadhyay <[email protected]>
---
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
Change logging of warnings to drm_warn() form dev_warn().
Signed-off-by: Suraj Upadhyay <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
> ---
> 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
Am 07.07.20 um 18:37 schrieb Suraj Upadhyay:
> Change logging information from dev_info() to drm_info().
>
> Signed-off-by: Suraj Upadhyay <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
> ---
> 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
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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
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 <[email protected]>
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
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
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
> > [email protected]
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
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__)
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.
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()
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.
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
> > > [email protected]
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >
> _______________________________________________
> dri-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/dri-devel