On 9/25/2017 3:31 PM, Joonas Lahtinen wrote:
On Fri, 2017-09-22 at 20:16 +0530, Sagar Arun Kamble wrote:
On 9/22/2017 5:34 PM, Joonas Lahtinen wrote:
On Fri, 2017-09-22 at 15:37 +0530, Sagar Arun Kamble wrote:
With GuC v9, new type of Default/critical logging in GuC to enable
capturing minimal important logs in production systems efficiently.
This patch enables this logging in GuC by default always. It should
be noted that streaming support with half-full interrupt mechanism
that is present for normal logging is not present for this type of
logging.
The commit message would be a good place to debrief the user impact. Do
we have the tools to capture the new style of log?
This has been verified to have minor impact by GuC team. Greg, Harsh can
clarify further.
Goal was to allow to get GuC logs from production systems.
These are the subset of same GuC logs available currently at all GuC log
levels and are needed to be captured
always whenever we want to capture GuC logs at i915.guc_log_level >=0
and <=3.
Chris had suggested on why this is not made on of the log levels and on
discussion with Harsh it is
concluded that this will not be supported as log level as GuC is using
fast functions to log the new
critical logs. Also there will not be half-full streaming support for
these type of logs.
I did have a patch earlier to disable this by default or emulate it as
log level to the end user but recommendation
from GuC team is to always turn this ON.
I see little point in detecting firmware version and unconditionally
setting a flag up at boot from kernel driver, when the firmware can do
it themselves knowing their own version.
We should not be polluting driver codebase with such constructs. It's
firmware, a new version can be provided easily unlike with hardware
which might need W/As.
Agree.
And question is why do we enable any logging by default, let it have
much or little impact on performance? This flag should probably be set
through debugfs or some other means when we know we're going to want
debugging output.
In my earlier patch we did have ability to turn this ON/OFF
https://patchwork.freedesktop.org/patch/173884/
Yeah, I think we might want to extend the existing .enable_guc_logging
parameter once we understand the relation between critical and regular
logging in a released firmware.
Could be as simple as;
(...).enable_critical_logging = (i915_modparams.guc_log_level >= 0);
For the time being, we shall wait for a fixed firmware.
Regards, Joonas
Sure. Will request for this change to be made in the firmware.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
