Message-ID: <20200716201926.GE2254583@ravnborg.org>
Date:   Thu, 16 Jul 2020 22:19:26 +0200
From:   Sam Ravnborg <sam@...nborg.org>
To:     Suraj Upadhyay <usuraj35@...il.com>
Cc:     maarten.lankhorst@...ux.intel.com, mripard@...nel.org,
        tzimmermann@...e.de, airlied@...ux.ie, daniel@...ll.ch,
        linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 0/4] drm: core: Convert logging to drm_* functions.

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@...ts.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 



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

Powered by blists - more mailing lists