Re: [PATCH v7.2-rc6] drm/amd/display: fix usage of DC_FPU_{BEGIN,END} with PREEMPT_RT

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2026-08-27 23:07:08 [+0200], Bert Karwatzki wrote:
> Am Donnerstag, dem 27.08.2026 um 14:49 +0200 schrieb Sebastian Andrzej Siewior:
> > On 2026-08-07 14:49:42 [+0200], Bert Karwatzki wrote:
> > > On PREEMPT_RT kernels kvzalloc_obj() can sleep because spin_lock is
> > > converted to rt_mutex. dc_create_plane_state() can be called while
> > > inside an FPU-guarded region, resuling in "scheduling while atomic"
> > > errors on PREEMPT_RT kernels.
> > >  Fix this by calling kvzalloc_obj() with DC_RUN_WITH_PREEMPTION_ENABLED().
> > > Also fix the error path in dc_create_stream_for_sink().
> > > 
> > > Fixes: 3539437f354b ("drm/amd/display: Move FPU Guards From DML To DC - Part 1")
> > > Link: https://lore.kernel.org/lkml/20260723123449.6494-1-spasswolf@web.de/
> > > Signed-off-by: Bert Karwatzki <spasswolf@web.de>
> > 
> > Thank you Bert. Got this somewhere in the meantime?
> 
> No, this is seems to be stuck somewhere ...

halp!

> > 
> > Could the FPU regions with disabled preemption be limited to where we
> > have actually have FPU usage in way that you don't have to worry when it
> > is needed to use DC_RUN_WITH_PREEMPTION_ENABLED() and when not? Also the
> > dc_fpu_begin()/ end() can nest and if they do the usage of
> > DC_RUN_WITH_PREEMPTION_ENABLED() is futile, isn't it?
> > 
> 
> I'm not very familiar with the amd display engine, and it's a lot of code,
> but there I think there's some room for improvement, e.g. dml2_destroy():
> 
> dml2_destroy is called from dc_state_free() from within DC_FP_{START,END}()
> and then basically just calls vfree with DC_RUN_WITH_PREEMPTION_ENABLED().
> (directly or through dml21_destroy()). Here both the DC_FP_*() tags and
> DC_RUN_WITH_PREEMPTION_ENABLED() could be dropped I think (not tested, yet)
> 
> Regarding nesting DC_FP_*()s: When I was searching for the cause of the
> kernel panics I monitored every DC_FP_{START,END}() with printk(), and I
> don't think I encountered nesting (the logs are lost unfortunately)

I am not saying it is nesting, just the START/STOP API is able to deal
this. Otherwise kernel_fpu_being/end() could be used directly. And
DC_RUN_WITH_PREEMPTION_ENABLED() is not able to deal with nesting.

On PREEMPT_RT it could be bad for the max observed latency if this nests
or has otherwise an unbound runtime. For crypto there an maximum amount
of input data which can be processed within a single FPU disabled
section. The "cheaper" second kernel_fpu_begin() makes it possible.

> > The first usage kernel_fpu_begin() saves the FPU state to the user task.
> > kernel_fpu_end() does not restore it. Therefore the subsequent
> > invocation of kernel_fpu_begin() is cheaper.
> > 
> > Sebastian
> 
> Bert Karwatzki

Sebastian