Re: [PATCH 0/4] drm/amd/powerplay: introduce the cgs print helpers

From: Huang Rui <ray.huang-5C7GfCeVMHo@public.gmane.org>
To: Grazvydas Ignotas <notasas-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: "Deucher,
	Alexander" <Alexander.Deucher-5C7GfCeVMHo@public.gmane.org>,
	"Nath, Arindam" <Arindam.Nath-5C7GfCeVMHo@public.gmane.org>,
	"Christian König"
	<deathsimple-ANTagKRnAhcb1SvskN2V4Q@public.gmane.org>,
	"Koenig,
	Christian" <Christian.Koenig-5C7GfCeVMHo@public.gmane.org>,
	"amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org"
	<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 0/4] drm/amd/powerplay: introduce the cgs print helpers
Date: Fri, 9 Dec 2016 10:29:51 +0800	[thread overview]
Message-ID: <20161209022950.GA11915@hr-amur2> (raw)
In-Reply-To: <CANOLnOOqGWKw5XdR-d=3GRrqWQ45KT_NJymWQW39kb0RtfK5Qw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Thu, Dec 08, 2016 at 09:04:13PM +0800, Grazvydas Ignotas wrote:
> On Thu, Dec 8, 2016 at 11:50 AM, Huang Rui <ray.huang@amd.com> wrote:
> > On Thu, Dec 08, 2016 at 05:27:30PM +0800, Christian König wrote:
> >> Am 08.12.2016 um 10:02 schrieb Huang Rui:
> >> > On Thu, Dec 08, 2016 at 04:41:04PM +0800, Koenig, Christian wrote:
> >> >> Sorry, but that just sounds like OS abstraction code which isn't allowed.
> >> >>
> >> >> There is no benefit except routing all messages through CGS which makes
> >> >> things much harder to follow.
> >> >>
> >> > There isn't COS part at current driver. But it seems to be not good to
> >> > introduce COS just for prints. Actually, most of drivers prefer to use
> >> > dev_* prints, and it's able to dynamic control the print level when we
> >> > debug it.
> >>
> >> Well I'm not sure if you have understood what I wanted to say.
> >>
> >> The reason that there isn't any COS abstraction is that it isn't allowed
> >> upstream.
> >>
> >
> > OK, I see.
> >
> >> Using the dev_* prints in the powerplay code is fine, but don't use the
> >> CGS or any other abstraction layer for them.
> >>
> >
> > Powerplay is quite independent component without amdgpu object, it is
> > hard to use dev_* prints without any abstraction layer.
> 
> Maybe you could use dev_set_name() with something powerplay related on
> relevant devices and then dev_* will print what you want?
> 

Garzvydas, thanks to your comments. dev_set_name function needs "struct
device" object in powerplay. But powerplay uses CGS layer to call into
"amdgpu" or "struct device". I think the intention is to make code porting
easier. So we won't bring any OS abstraction code into powerplay.

> Alternatively you could do
> 
> #define pr_fmt(fmt) "[powerplay] " fmt
> 
> before #include <linux/...> and then all pr_* functions will prefix
> their messages.
> 

pr_* actually is almost the same with printk, but it seems fine here to
just add a prefix.

Thanks,
Rui
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx