Re: drm/i915: Watchdog timeout: IRQ handler for gen8+

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

 




On 07/01/2019 13:02, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-01-07 12:58:39)

On 07/01/2019 12:16, Chris Wilson wrote:
Quoting Tvrtko Ursulin (2019-01-07 11:58:13)
On 05/01/2019 02:39, Carlos Santa wrote:
+static void gen8_watchdog_irq_handler(unsigned long data)
+{
+     struct intel_engine_cs *engine = (struct intel_engine_cs *)data;
+     struct drm_i915_private *dev_priv = engine->i915;
+     enum forcewake_domains fw_domains;
+     u32 current_seqno;
+
+     switch (engine->id) {
+     default:
+             MISSING_CASE(engine->id);
+             /* fall through */
+     case RCS:
+             fw_domains = FORCEWAKE_RENDER;
+             break;
+     case VCS:
+     case VCS2:
+     case VECS:
+             fw_domains = FORCEWAKE_MEDIA;
+             break;
+     }
+
+     intel_uncore_forcewake_get(dev_priv, fw_domains);

I'd be tempted to drop this and just use I915_WRITE. It doesn't feel
like there is any performance to be gained with it and it embeds too
much knowledge here.

No, no, no. Let's not reintroduce a fw inside irq context on a frequent
timer again.

Tasklet and hopefully watchdog timeouts are not frequent. :)

I thought the typical value mentioned elsewhere was a 1ms watchdog. Some

Commit message to a patch from this series says 60ms is recommended.

might say why even use a watchdog for longer than that as a hrtimer will
be more efficient (coupling in with other timer activity) ;)

For the normal case (longish batches, very few timeouts) it feels more efficient to have ten or so extra dwords with each request than to fiddle with hrtimers, no?

But interactions with preemption and future time-slicing yeah don't know. Maybe a single solution will be simpler at that point.

Regards,

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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux