On Tue, May 12, 2015 at 01:02:08PM +0530, Animesh Manna wrote: > Specifically csr mutex lock is to protect csr-related data structures > so declaration moved intel_csr structure. > > Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx> > Signed-off-by: A.Sunil Kamath <sunil.kamath@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_dma.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 6 +++--- > drivers/gpu/drm/i915/intel_csr.c | 12 ++++++------ > 3 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index a238889..78e6ae8 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -816,7 +816,7 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) > spin_lock_init(&dev_priv->mmio_flip_lock); > mutex_init(&dev_priv->dpio_lock); > mutex_init(&dev_priv->modeset_restore_lock); > - mutex_init(&dev_priv->csr_lock); > + mutex_init(&dev_priv->csr.csr_lock); > > intel_pm_setup(dev); > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 136d42a..43011d7 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -676,6 +676,9 @@ enum csr_state { > }; > > struct intel_csr { > + /* CSR protection, used to protect firmware loading status: csr_state */ > + struct mutex csr_lock; csr_ prefix is redundant. But the real trouble is that essentially your open-coding a completion using this mutex and csr->state. And that's just confusing compared to using completions directly. Can you please replace the usage of csr->state with a completion and remove the mutex entirely? -Daniel > + > const char *fw_path; > __be32 *dmc_payload; > uint32_t dmc_fw_size; > @@ -1592,9 +1595,6 @@ struct drm_i915_private { > > struct intel_csr csr; > > - /* Display CSR-related protection */ > - struct mutex csr_lock; > - > struct intel_gmbus gmbus[GMBUS_NUM_PINS]; > > /** gmbus_mutex protects against concurrent usage of the single hw gmbus > diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > index 174106f..b03b9b3 100644 > --- a/drivers/gpu/drm/i915/intel_csr.c > +++ b/drivers/gpu/drm/i915/intel_csr.c > @@ -195,9 +195,9 @@ enum csr_state intel_csr_load_status_get(struct drm_i915_private *dev_priv) > { > enum csr_state state; > > - mutex_lock(&dev_priv->csr_lock); > + mutex_lock(&dev_priv->csr.csr_lock); > state = dev_priv->csr.state; > - mutex_unlock(&dev_priv->csr_lock); > + mutex_unlock(&dev_priv->csr.csr_lock); > > return state; > } > @@ -212,9 +212,9 @@ enum csr_state intel_csr_load_status_get(struct drm_i915_private *dev_priv) > void intel_csr_load_status_set(struct drm_i915_private *dev_priv, > enum csr_state state) > { > - mutex_lock(&dev_priv->csr_lock); > + mutex_lock(&dev_priv->csr.csr_lock); > dev_priv->csr.state = state; > - mutex_unlock(&dev_priv->csr_lock); > + mutex_unlock(&dev_priv->csr.csr_lock); > } > > /** > @@ -236,7 +236,7 @@ void intel_csr_load_program(struct drm_device *dev) > return; > } > > - mutex_lock(&dev_priv->csr_lock); > + mutex_lock(&dev_priv->csr.csr_lock); > fw_size = dev_priv->csr.dmc_fw_size; > for (i = 0; i < fw_size; i++) > I915_WRITE(CSR_PROGRAM_BASE + i * 4, > @@ -248,7 +248,7 @@ void intel_csr_load_program(struct drm_device *dev) > } > > dev_priv->csr.state = FW_LOADED; > - mutex_unlock(&dev_priv->csr_lock); > + mutex_unlock(&dev_priv->csr.csr_lock); > } > > static void finish_csr_load(const struct firmware *fw, void *context) > -- > 2.0.2 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx