On Mon, Sep 07, 2015 at 04:34:30PM +0530, Sunil Kamath wrote: > On Wednesday 26 August 2015 01:36 AM, Animesh Manna wrote: > >Dmc will restore the csr program except DC9, cold boot, > >warm reset, PCI function level reset, and hibernate/suspend. > > > >intel_csr_load_program() function is used to load the firmware > >data from kernel memory to csr address space. > > > >All values of csr address space will be zero if it got reset and > >the first byte of csr program is always a non-zero if firmware > >is loaded successfuly. Based on hardware status will load the > >firmware. > > > >Without this condition check if we overwrite the firmware data the > >counters exposed for dc5/dc6 (help for debugging) will be nullified. > > > >v1: Initial version. > > > >v2: Based on review comments from Daniel, > >- Added a check to know hardware status and load the firmware if not loaded. > > > >Cc: Daniel Vetter <daniel.vetter@xxxxxxxxx> > >Cc: Damien Lespiau <damien.lespiau@xxxxxxxxx> > >Cc: Imre Deak <imre.deak@xxxxxxxxx> > >Cc: Sunil Kamath <sunil.kamath@xxxxxxxxx> > >Signed-off-by: Animesh Manna <animesh.manna@xxxxxxxxx> > >Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@xxxxxxxxx> > >--- > > drivers/gpu/drm/i915/intel_csr.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > >diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c > >index ba1ae03..682cc26 100644 > >--- a/drivers/gpu/drm/i915/intel_csr.c > >+++ b/drivers/gpu/drm/i915/intel_csr.c > >@@ -252,6 +252,15 @@ void intel_csr_load_program(struct drm_device *dev) > > return; > > } > >+ /* > >+ * Dmc will restore the csr the program except DC9, cold boot, > >+ * warm reset, PCI function level reset, and hibernate/suspend. > >+ * This condition will help to check if csr address space is reset/ > >+ * not loaded. > >+ */ > >+ if (I915_READ(CSR_PROGRAM_BASE)) > >+ return; > >+ > > mutex_lock(&dev_priv->csr_lock); > > fw_size = dev_priv->csr.dmc_fw_size; > > for (i = 0; i < fw_size; i++) > > Valid fix and patch is ready for merge now. > > Reviewed-by: A.Sunil Kamath <sunil.kamath@xxxxxxxxx> Ok I changed the code comment to a FIXME to explain what's going on (DC9 is totally irrelevant if I understand this correctly now) and amended the commit message with a note about what I think is really going on. Please double-check. Also I realized that we have 0 test coverage for suspend-to-idle (and oversight for merging this feature a few years back). Please add a new igt testcase (maybe subtest of drv_suspend) to exercise this feature. Making sure that we have sufficient igt coverage is part of the review and especially something that _must_ be checked for bugfixes like this. Thanks, Daniel -- 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
