Re: [Intel-gfx] [PATCH] drm/i915: Depend on !PREEMPT_RT.

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 28/02/2022 10:35, Sebastian Andrzej Siewior wrote:
> On 2022-02-28 10:10:48 [+0000], Tvrtko Ursulin wrote:
>> Hi,
> Hi,
> 
>> Could you paste a link to the queue of i915 patches pending for a quick
>> overview of how much work there is and in what areas?
> 
> Last post to the list:
>    https://https://lkml.kernel.org/r/.kernel.org/all/20211214140301.520464-1-bigeasy@linutronix.de/
> 
> or if you look at the DRM section in
>     https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git/tree/patches/series?h=v5.17-rc6-rt10-patches#n156

Thanks!

> you see:
>     0003-drm-i915-Use-preempt_disable-enable_rt-where-recomme.patch
>     0004-drm-i915-Don-t-disable-interrupts-on-PREEMPT_RT-duri.patch

Two for the display folks.

>     0005-drm-i915-Don-t-check-for-atomic-context-on-PREEMPT_R.patch

What do preempt_disable/enable do on PREEMPT_RT? Thinking if instead the 
solution could be to always force the !ATOMIC path (for the whole 
_wait_for_atomic macro) on PREEMPT_RT.

>     0006-drm-i915-Disable-tracing-points-on-PREEMPT_RT.patch

If the issue is only with certain trace points why disable all?

>     0007-drm-i915-skip-DRM_I915_LOW_LEVEL_TRACEPOINTS-with-NO.patch

Didn't quite fully understand, why is this not fixable? Especially 
thinking if the option of not blanket disabling all tracepoints in the 
previous patch.

>     0008-drm-i915-gt-Queue-and-wait-for-the-irq_work-item.patch

Not sure about why cond_resched was put between irq_work_queue and 
irq_work_sync - would it not be like-for-like change to have the two 
together? Commit message makes me think _queue already starts the 
handler on x86 at least.

>     0009-drm-i915-gt-Use-spin_lock_irq-instead-of-local_irq_d.patch

I think this is okay. The part after the unlock is serialized by the 
tasklet already.

Slight doubt due the comment:

   local_irq_enable(); /* flush irq_work (e.g. breadcrumb enabling) */

Makes me want to think about it harder but not now.

Another thing to check is if execlists_context_status_change still needs 
the atomic notifier chain with this change.

>     0010-drm-i915-Drop-the-irqs_disabled-check.patch

LGTM.

>     Revert-drm-i915-Depend-on-PREEMPT_RT.patch

Okay.

And finally for this very patch (the thread I am replying to):

Acked-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

Tvrtko

> 
> and you could view them from
>     https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-rt-devel.git/tree/patches?h=v5.17-rc6-rt10-patches
> 
>> Also, I assume due absence of ARCH_SUPPORTS_RT being defined by any arch,
>> that something more is not yet ready?
> 
> Correct. Looking at what I have queued for the next merge window I have
> less than 20 patches (excluding i915 and printk) before ARCH_SUPPORTS_RT
> can be enabled for x86-64.
>   
>> Regards,
>>
>> Tvrtko
> 
> Sebastian