[Intel-xe] [PATCH] drm/xe/irq: the irq handler local variable need not be static

[Lucas De Marchi]

On Tue, Mar 14, 2023 at 12:32:59PM +0200, Jani Nikula wrote:
>On Mon, 13 Mar 2023, Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>> On Thu, Mar 09, 2023 at 09:37:34AM -0800, Matt Roper wrote:
>>>On Thu, Mar 09, 2023 at 02:21:33PM +0200, Jani Nikula wrote:
>>>> It's just a local variable.
>>>>
>>>> Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>>>
>>>Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
>>
>>
>> Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>
>> but the commit message should probably say 's/need/should/'. Setting it
>> as static pretty much breaks setups with 2 devices like TGL + DG2.
>
>Mmmh, only if the probe races between xe_irq_install() calls for the two
>devices. Can the same driver probe two devices in parallel?

true, I don't think currently it will ever be in parallel since 
we are tying the bind to the module load. We could stimulate
a race by doing the bind separately, but even then the window for
the race is just a few instructions.

When I read this commit I was under the impression that whatever got set
in this variable would end up being used later. Checking again, it's
only used inside the xe_irq_install to call directly.

So I'm fine with the current message and my r-b stands.

thanks
Lucas De Marchi

>
>BR,
>Jani.
>
>
>>
>> Lucas De Marchi
>>
>>>
>>>> ---
>>>>  drivers/gpu/drm/xe/xe_irq.c | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/xe/xe_irq.c b/drivers/gpu/drm/xe/xe_irq.c
>>>> index ae2f65c00fa6..529b42d9c9af 100644
>>>> --- a/drivers/gpu/drm/xe/xe_irq.c
>>>> +++ b/drivers/gpu/drm/xe/xe_irq.c
>>>> @@ -529,7 +529,7 @@ static void irq_uninstall(struct drm_device *drm, void *arg)
>>>>  int xe_irq_install(struct xe_device *xe)
>>>>  {
>>>>  	int irq = to_pci_dev(xe->drm.dev)->irq;
>>>> -	static irq_handler_t irq_handler;
>>>> +	irq_handler_t irq_handler;
>>>>  	int err;
>>>>
>>>>  	irq_handler = xe_irq_handler(xe);
>>>> --
>>>> 2.39.1
>>>>
>>>
>>>--
>>>Matt Roper
>>>Graphics Software Engineer
>>>Linux GPU Platform Enablement
>>>Intel Corporation
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center