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. > > 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 -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
