On 08/17/2016 09:11 AM, Sean Paul wrote: > The delayed worker isn't needed and is racey. Remove it and do > the state change in line. > > Signed-off-by: Sean Paul <seanpaul at chromium.org> Reviewed-by: Yakir Yang <ykk at rock-chips.com> Tested-by: Yakir Yang <ykk at rock-chips.com> > --- > > Changes in v2: > - Rebased on https://cgit.freedesktop.org/~seanpaul/dogwood/log/?h=for-next > > drivers/gpu/drm/rockchip/rockchip_drm_psr.c | 38 ++++++++--------------------- > 1 file changed, 10 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > index bd25273..4c645d7 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_psr.c > @@ -19,7 +19,6 @@ > #include "rockchip_drm_psr.h" > > #define PSR_FLUSH_TIMEOUT msecs_to_jiffies(3000) /* 3 seconds */ > -#define PSR_SET_DELAY_TIME msecs_to_jiffies(10) > > enum psr_state { > PSR_FLUSH, > @@ -31,11 +30,8 @@ struct psr_drv { > struct list_head list; > struct drm_encoder *encoder; > > - enum psr_state request_state; > enum psr_state state; > > - struct delayed_work state_work; > - > struct timer_list flush_timer; > > void (*set)(struct drm_encoder *encoder, bool enable); > @@ -59,11 +55,8 @@ out: > return psr; > } > > -static void psr_state_work(struct work_struct *work) > +static void psr_set_state(struct psr_drv *psr, enum psr_state state) > { > - struct psr_drv *psr = container_of(work, typeof(*psr), state_work.work); > - enum psr_state request_state = psr->request_state; > - > /* > * Allowed finite state machine: > * > @@ -75,24 +68,22 @@ static void psr_state_work(struct work_struct *work) > */ > > /* Forbid no state change */ > - if (request_state == psr->state) > + if (state == psr->state) > return; > > /* Forbid DISABLE change to FLUSH */ > - if (request_state == PSR_FLUSH && psr->state == PSR_DISABLE) > + if (state == PSR_FLUSH && psr->state == PSR_DISABLE) > return; > > + /* Only wrote in this work, no need lock protection */ > + psr->state = state; > + > /* Allow but no need hardware change, just need assign the state */ > - if (request_state == PSR_DISABLE && psr->state == PSR_FLUSH) { > - psr->state = request_state; > + if (state == PSR_DISABLE && psr->state == PSR_FLUSH) > return; > - } > - > - /* Only wrote in this work, no need lock protection */ > - psr->state = request_state; > > /* Refact to hardware state change */ > - switch (request_state) { > + switch (psr->state) { > case PSR_ENABLE: > psr->set(psr->encoder, true); > break; > @@ -104,13 +95,6 @@ static void psr_state_work(struct work_struct *work) > } > } > > -static void psr_set_state(struct psr_drv *psr, enum psr_state state) > -{ > - psr->request_state = state; > - > - schedule_delayed_work(&psr->state_work, PSR_SET_DELAY_TIME); > -} > - > static void psr_flush_handler(unsigned long data) > { > struct psr_drv *psr = (struct psr_drv *)data; > @@ -119,7 +103,7 @@ static void psr_flush_handler(unsigned long data) > return; > > /* State changed between flush time, then keep it */ > - if (psr->request_state != PSR_FLUSH) > + if (psr->state != PSR_FLUSH) > return; > > psr_set_state(psr, PSR_ENABLE); > @@ -183,7 +167,7 @@ void rockchip_drm_psr_flush(struct drm_device *dev) > > spin_lock_irqsave(&drm_drv->psr_list_lock, flags); > list_for_each_entry(psr, &drm_drv->psr_list, list) { > - if (psr->request_state == PSR_DISABLE) > + if (psr->state == PSR_DISABLE) > continue; > > mod_timer(&psr->flush_timer, > @@ -219,8 +203,6 @@ int rockchip_drm_psr_register(struct drm_encoder *encoder, > > setup_timer(&psr->flush_timer, psr_flush_handler, (unsigned long)psr); > > - INIT_DELAYED_WORK(&psr->state_work, psr_state_work); > - > psr->state = PSR_DISABLE; > psr->encoder = encoder; > psr->set = psr_set;
