On 20/07/16 12:29, Goel, Akash wrote:
On 7/20/2016 4:10 PM, Tvrtko Ursulin wrote:
On 20/07/16 11:12, Goel, Akash wrote:
On 7/20/2016 3:17 PM, Tvrtko Ursulin wrote:
+ ret = -EINVAL;
+ goto end;
+ }
+
+ intel_runtime_pm_get(dev_priv);
+ ret = i915_guc_log_control(dev, val);
+ intel_runtime_pm_put(dev_priv);
+
+end:
+ mutex_unlock(&dev->struct_mutex);
+ return ret;
+}
+
+DEFINE_SIMPLE_ATTRIBUTE(i915_guc_log_control_fops,
+ NULL, i915_guc_log_control_set,
+ "0x%08llx\n");
Does the readback still work with no get method?
readback will give a 'Permission denied' error
Is that what we want? I think it would be nice to allow read-back
unless
there is a specific reason why it shouldn't be allowed.
Ok can implement a dummy read back function but what should be
shown/returned on read.
Should I show/return the guc_log_level value (which is also available
from /sys/module/i915/parameters/) ?
I would return the same value that was written in. Is the problem that
it is not stored anywhere? Maybe reconstruct it from
i915.guc_log_level ?
The verbosity value will be same as guc_log_level. But whether logging
on GuC side is currently enabled or disabled can't be inferred (it could
have been disabled at run time).
So will have to store the exact value written by User.
That's what I meant. Code currently seem to decompose the value written
via debugfs and store it in i915.guc_log_level:
0x00 = -1
0x10 = -1
...
0x01 = 0
0x11 = 1
0x21 = 2
0x31 = 3
...
So for readback you could translate back from i915.guc_log_level to the
debugfs format.
Sorry for all the mess.
Should I add a new field 'debugfs_ctrl_val' in guc structure, to store
the value previously written to debugfs file, considering guc_log_level
only gives an indication of the verbosity level ?
Actually in future there may be other additions also to the value
written to guc_log_control debugfs, have right now exposed only logging
& verbosity level controls to User, as they are deemed most useful right
now.
But there are some other controls also which can be passed to GuC
firmware through UK_LOG_ENABLE_LOGGING host2guc action.
I see.
Would it work, for time being at least, to set i915.guc_log_level to -1
when logging is disabled via debugfs?
It think that also has the advantage of making the current guc logging
state consistent when observed from the outside. Otherwise the debugfs
value and module parameter may disagree on it, as you said before. Which
is not that great I think.
Apart from making the reported stated consistent, that way you could, at
least for the time being, get away without storing a copy of
guc_log_control but reconstruct it from the module parameter on read-back.
Regards,
Tvrtko
You could avoid storing a copy of guc_log_control like that.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
