On Thu, Jun 6, 2019 at 7:28 PM Daniel Vetter <daniel.vetter@xxxxxxxx> 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@xxxxxxxxx>
> Cc: Rodrigo Siqueira <rodrigosiqueiramelo@xxxxxxxxx>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> Cc: Haneen Mohammed <hamohammed.sa@xxxxxxxxx>
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> ---
> 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@xxxxxxxxx>
Tested-by: Rodrigo Siqueira <rodrigosiqueiramelo@xxxxxxxxx>
> + spin_unlock_irq(&out->lock);
>
> return ret;
> }
> --
> 2.20.1
>
--
Rodrigo Siqueira
https://siqueira.tech
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
