Re: [RFC 1/6] drm/i915: cover ioctls with runtime_get/put

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 02/22/2014 01:37 AM, Jesse Barnes wrote:
On Fri, 24 Jan 2014 19:23:54 +0200
Imre Deak <imre.deak@xxxxxxxxx> wrote:

On Fri, 2014-01-24 at 20:35 +0530, Naresh Kumar Kachhi wrote:
On 01/22/2014 06:53 PM, Imre Deak wrote:

On Wed, 2014-01-22 at 13:51 +0100, Daniel Vetter wrote:
On Wed, Jan 22, 2014 at 05:34:17PM +0530, naresh.kumar.kachhi@xxxxxxxxx wrote:
From: Naresh Kumar Kachhi <naresh.kumar.kachhi@xxxxxxxxx>

With runtime PM enabled, we need to make sure that all HW access
are valid (i.e.  Gfx is in D0). Invalid accesses might end up in
HW hangs. Ex. A hang is seen if display register is accessed on
BYT while display power island is power gated.

This patch is covering all the IOCTLs with get/put.
TODO: limit runtime_get/put to IOCTLs that accesses HW

Signed-off-by: Naresh Kumar Kachhi <naresh.kumar.kachhi@xxxxxxxxx>
Nack on the concept. We need to have get/put calls for the individual
functional blocks of the hw, which then in turn (if it's not the top-level
power domain) need to grab references to the next up power domain.
Splattering unconditional get/puts over all driver entry points is bad
style and imo also too fragile.

Also, with Paulos final runtime pm/pc8 unification patches and Imre's
display power well patches for byt we should have full coverage already.
Have you looked at the work of these too?
I'm still in debt with the BYT specific power domain patches, I want to
post them (this week) after I sorted out spurious pipe stat IRQs we'd
receive atm with the power well off. Until then there is only the WIP
version at: https://github.com/ideak/linux/commits/powerwells

But in practice, as you point out the plan was to only call
modeset_update_power_wells() during modeset time and that will end up
doing the proper RPM get/put as necessary. Similarly on some other code
paths like connector detect and HW state read-out code, we'd ask only
for the needed power domains which would end up doing the RPM get/put.

--Imre

Hi Imre,
I tried to go through your and Paulo's patches (http://patchwork.freedesktop.org/patch/16952/).
As per my understanding we are doing disp power well gate/ungate independent of we are in
runtime_suspend/resume state we might face problem here as on BYT interrupts are routed through
display power well, so we might have a situation where display power island is power gated,
but render workload is still active. As we won't be receiving any interrupt __wait_seq_no will
stall and we might end up in a TDR. We will not see the issue until display power gating is
enabled. I will try to include a test to cover this test
Hm, which exact interrupt routing are you referring to? At least on my
BYT I manage to power off only the display power well and keep the
render well on, while still being able to run for example
igt/tests/gem_render_copy, gem_render_linear_blits. I can see that
through /proc/interrupts that GT interrupts are properly generated. The
display side interrupt routing is of course off, for example the
PIPESTAT registers will read all 0xffffffff, but DEIIR, DEIMR, DEISR are
all seem to work ok.
I think Naresh is referring to full D3 state shutdown, where the Gunit
goes away too.  But in that case the GT will be shut down as well, so I
don't think the display vs GT thing by itself is a problem.

Please ignore my previous comment as I confused interrupts being routed through display unit on VLV.
But this comment will be valid on some platforms like HSW, where render interrupts are routed
through display unit. So, display power gating in such platforms has to be done by taking
render/media also into consideration.
-Naresh

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux