From: Sam Ravnborg <sam@ravnborg.org> To: Suraj Upadhyay <usuraj35@gmail.com> Cc: maarten.lankhorst@linux.intel.com, mripard@kernel.org, tzimmermann@suse.de, airlied@linux.ie, daniel@ffwll.ch, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions. Date: Thu, 16 Jul 2020 22:19:26 +0200 [thread overview] Message-ID: <20200716201926.GE2254583@ravnborg.org> (raw) In-Reply-To: <20200711151126.GA12262@blackclown> 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
WARNING: multiple messages have this Message-ID (diff)
From: Sam Ravnborg <sam@ravnborg.org> To: Suraj Upadhyay <usuraj35@gmail.com> Cc: tzimmermann@suse.de, airlied@linux.ie, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions. Date: Thu, 16 Jul 2020 22:19:26 +0200 [thread overview] Message-ID: <20200716201926.GE2254583@ravnborg.org> (raw) In-Reply-To: <20200711151126.GA12262@blackclown> 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 _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
next prev parent reply other threads:[~2020-07-16 20:19 UTC|newest] Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-07 16:34 [PATCH 0/4] drm: core: Convert logging to drm_* functions Suraj Upadhyay 2020-07-07 16:34 ` Suraj Upadhyay 2020-07-07 16:28 ` [PATCH 1/4] drm: mipi-dsi: " Suraj Upadhyay 2020-07-07 16:35 ` Suraj Upadhyay 2020-07-07 16:35 ` Suraj Upadhyay 2020-07-07 16:28 ` Suraj Upadhyay 2020-07-10 18:01 ` Sam Ravnborg 2020-07-10 18:01 ` Sam Ravnborg 2020-07-07 16:36 ` [PATCH 2/4] drm: mipi-dbi: " Suraj Upadhyay 2020-07-07 16:36 ` Suraj Upadhyay 2020-07-07 16:36 ` [PATCH 3/4] drm: edid: " Suraj Upadhyay 2020-07-07 16:36 ` Suraj Upadhyay 2020-07-08 6:10 ` Thomas Zimmermann 2020-07-08 6:10 ` Thomas Zimmermann 2020-07-07 16:37 ` [PATCH 4/4] drm: fb-helper: " Suraj Upadhyay 2020-07-07 16:37 ` Suraj Upadhyay 2020-07-08 6:10 ` Thomas Zimmermann 2020-07-08 6:10 ` Thomas Zimmermann 2020-07-10 18:02 ` Sam Ravnborg 2020-07-10 18:02 ` Sam Ravnborg 2020-07-10 17:56 ` [PATCH 0/4] drm: core: " Sam Ravnborg 2020-07-10 17:56 ` Sam Ravnborg 2020-07-11 15:11 ` Suraj Upadhyay 2020-07-11 15:11 ` Suraj Upadhyay 2020-07-11 18:16 ` Joe Perches 2020-07-11 18:16 ` Joe Perches 2020-07-12 18:54 ` Suraj Upadhyay 2020-07-12 18:54 ` Suraj Upadhyay 2020-07-12 19:07 ` Joe Perches 2020-07-12 19:07 ` Joe Perches 2020-07-13 13:58 ` Suraj Upadhyay 2020-07-13 13:58 ` Suraj Upadhyay 2020-07-16 20:19 ` Sam Ravnborg [this message] 2020-07-16 20:19 ` Sam Ravnborg
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200716201926.GE2254583@ravnborg.org \ --to=sam@ravnborg.org \ --cc=airlied@linux.ie \ --cc=daniel@ffwll.ch \ --cc=dri-devel@lists.freedesktop.org \ --cc=linux-kernel@vger.kernel.org \ --cc=maarten.lankhorst@linux.intel.com \ --cc=mripard@kernel.org \ --cc=tzimmermann@suse.de \ --cc=usuraj35@gmail.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.