[PATCH 02/10] drm/vkms: Use spin_lock_irq in process context

[Rodrigo Siqueira]

On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>
> The worker is always in process context, no need for the _irqsafe
> version. Same for the set_source callback, that's only called from the
> debugfs handler in a syscall.
>
> Cc: Shayenne Moura <shayenneluzmoura at gmail.com>
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo at gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> Cc: Haneen Mohammed <hamohammed.sa at gmail.com>
> Cc: Daniel Vetter <daniel at ffwll.ch>
> ---
>  drivers/gpu/drm/vkms/vkms_crc.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_crc.c b/drivers/gpu/drm/vkms/vkms_crc.c
> index 66603da634fe..883e36fe7b6e 100644
> --- a/drivers/gpu/drm/vkms/vkms_crc.c
> +++ b/drivers/gpu/drm/vkms/vkms_crc.c
> @@ -167,16 +167,15 @@ void vkms_crc_work_handle(struct work_struct *work)
>         u32 crc32 = 0;
>         u64 frame_start, frame_end;
>         bool crc_pending;
> -       unsigned long flags;
>
> -       spin_lock_irqsave(&out->state_lock, flags);
> +       spin_lock_irq(&out->state_lock);
>         frame_start = crtc_state->frame_start;
>         frame_end = crtc_state->frame_end;
>         crc_pending = crtc_state->crc_pending;
>         crtc_state->frame_start = 0;
>         crtc_state->frame_end = 0;
>         crtc_state->crc_pending = false;
> -       spin_unlock_irqrestore(&out->state_lock, flags);
> +       spin_unlock_irq(&out->state_lock);
>
>         /*
>          * We raced with the vblank hrtimer and previous work already computed
> @@ -246,7 +245,6 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name)
>  {
>         struct vkms_output *out = drm_crtc_to_vkms_output(crtc);
>         bool enabled = false;
> -       unsigned long flags;
>         int ret = 0;
>
>         ret = vkms_crc_parse_source(src_name, &enabled);
> @@ -254,9 +252,9 @@ int vkms_set_crc_source(struct drm_crtc *crtc, const char *src_name)
>         /* make sure nothing is scheduled on crtc workq */
>         flush_workqueue(out->crc_workq);
>
> -       spin_lock_irqsave(&out->lock, flags);
> +       spin_lock_irq(&out->lock);
>         out->crc_enabled = enabled;
> -       spin_unlock_irqrestore(&out->lock, flags);

I was wondering if we could use atomic_t for crc_enabled and avoid
this sort of lock. I did not try to change the data type; this is just
an idea.

Reviewed-by: Rodrigo Siqueira <rodrigosiqueiramelo at gmail.com>
Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo at gmail.com>

> +       spin_unlock_irq(&out->lock);
>
>         return ret;
>  }
> --
> 2.20.1
>


-- 

Rodrigo Siqueira
https://siqueira.tech