On Thu, 08 Dec 2022, Andrzej Hajda <[email protected]> wrote: > Simplify the code.
Personally, I absolutely hate fetch_and_zero(). I understand the point, but there are two main traps: First, the name implies atomicity, which there is none at all. Second, the name implies it's part of a kernel core header, which it isn't, and this just amplifies the first point. It's surprising and misleading, and those are not things I like about interfaces in the kernel. I would not like to see this proliferate. If fetch_and_zero() was atomic *and* part of a core kernel header, it would be a different matter. But I don't think that's going to happen, exactly because it won't be atomic and the name implies it is. BR, Jani. > > Signed-off-by: Andrzej Hajda <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_hotplug.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_hotplug.c > b/drivers/gpu/drm/i915/display/intel_hotplug.c > index 907ab7526cb478..2972d7533da44e 100644 > --- a/drivers/gpu/drm/i915/display/intel_hotplug.c > +++ b/drivers/gpu/drm/i915/display/intel_hotplug.c > @@ -304,10 +304,8 @@ static void i915_digport_work_func(struct work_struct > *work) > u32 old_bits = 0; > > spin_lock_irq(&dev_priv->irq_lock); > - long_port_mask = dev_priv->display.hotplug.long_port_mask; > - dev_priv->display.hotplug.long_port_mask = 0; > - short_port_mask = dev_priv->display.hotplug.short_port_mask; > - dev_priv->display.hotplug.short_port_mask = 0; > + long_port_mask = > fetch_and_zero(&dev_priv->display.hotplug.long_port_mask); > + short_port_mask = > fetch_and_zero(&dev_priv->display.hotplug.short_port_mask); > spin_unlock_irq(&dev_priv->irq_lock); > > for_each_intel_encoder(&dev_priv->drm, encoder) { > @@ -379,10 +377,8 @@ static void i915_hotplug_work_func(struct work_struct > *work) > > spin_lock_irq(&dev_priv->irq_lock); > > - hpd_event_bits = dev_priv->display.hotplug.event_bits; > - dev_priv->display.hotplug.event_bits = 0; > - hpd_retry_bits = dev_priv->display.hotplug.retry_bits; > - dev_priv->display.hotplug.retry_bits = 0; > + hpd_event_bits = fetch_and_zero(&dev_priv->display.hotplug.event_bits); > + hpd_retry_bits = fetch_and_zero(&dev_priv->display.hotplug.retry_bits); > > /* Enable polling for connectors which had HPD IRQ storms */ > intel_hpd_irq_storm_switch_to_polling(dev_priv); -- Jani Nikula, Intel Open Source Graphics Center
