From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sunil Kamath Subject: Re: [DMC_BUGFIX_SKL_V2 1/5] drm/i915/skl: Added a check for the hardware status of csr fw before loading. Date: Mon, 07 Sep 2015 16:34:30 +0530 Message-ID: <55ED6F3E.7030205@intel.com> References: <1440533169-32265-1-git-send-email-animesh.manna@intel.com> <1440533169-32265-2-git-send-email-animesh.manna@intel.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0539337463==" Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id D02F3720DC for ; Mon, 7 Sep 2015 04:04:33 -0700 (PDT) In-Reply-To: <1440533169-32265-2-git-send-email-animesh.manna@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Animesh Manna Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org This is a multi-part message in MIME format. --===============0539337463== Content-Type: multipart/alternative; boundary="------------010908020508090207050602" This is a multi-part message in MIME format. --------------010908020508090207050602 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 > Cc: Damien Lespiau > Cc: Imre Deak > Cc: Sunil Kamath > Signed-off-by: Animesh Manna > Signed-off-by: Vathsala Nagaraju > --- > 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 --------------010908020508090207050602 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit
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@intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Sunil Kamath <sunil.kamath@intel.com>
Signed-off-by: Animesh Manna <animesh.manna@intel.com>
Signed-off-by: Vathsala Nagaraju <vathsala.nagaraju@intel.com>
---
 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@intel.com>

--------------010908020508090207050602-- --===============0539337463== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4 IG1haWxpbmcgbGlzdApJbnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK --===============0539337463==--