On Mon, Mar 07, 2016 at 08:57:09PM +0200, ville.syrjala@xxxxxxxxxxxxxxx wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> Read out the RPS frequencies already in intel_init_gt_powersave()
> on all the platforms. So far we only did that on VLV/CHV, and the
> rest of the platforms read them out at rps enable time, which happens
> asynchronously from a workqueue. Reading them out earlier prevents
> userspace from reading out invalid (zero) values via the relevant
> sysfs files before the rps enable work has been executed.
>
> This used to be prevented by the flush_delayed_work() + locking
> in the sysfs code, but now that we no longer do that, we run the
> risk of letting userspace see the initial zeroed values.
>
> Note that it's still possible to read out cur_freq as 0, since that
> only gets initialized from the delayed rps enable. Should that pose
> a real problem, I guess we could always add the flush+locking back
> for the cur_freq read.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> ---
Hello,
The flush_delayed_work() calls were added in:
"commit 5c9669cee534cbb834d51aae115267f5e561b622
Author: Tom O'Rourke <Tom.O'Rourke@xxxxxxxxx>
Date: Mon Sep 16 14:56:43 2013 -0700
drm/i915: Finish enabling rps before use by sysfs or debugfs"
This change was made to address use before initialization
problems in min/max/cur frequency values. The work queue
being flushed was added in:
"commit 1a01ab3b2dc4394c46c4c3230805748f632f6f74
Author: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>
Date: Fri Nov 2 11:14:00 2012 -0700
drm/i915: put ring frequency and turbo setup into a work queue v5
Communicating via the mailbox registers with the PCU can take quite
awhile. And updating the ring frequency or enabling turbo is not
something that needs to happen synchronously, so take it out of our init
and resume paths to speed things up (~200ms on my T420)."
This change was made to reduce driver load times.
The two concerns I have are:
1) Similar changes would be needed in debugfs files.
2) This change may increase driver load times due to pcode
mailbox command used in gen6_rps_init_frequencies() that
is now in the init path.
I am not objecting to these changes but we should
be aware of the impact to driver load latency.
Thanks,
Tom
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
