diff mbox

[SKL-DMC-BUGFIX,1/5] drm/i915/gen9: Removed byte swapping for csr firmware

Message ID 1438619136-18268-2-git-send-email-animesh.manna@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Manna, Animesh Aug. 3, 2015, 4:25 p.m. UTC
This patch contains the changes to remove the byte
swapping logic introduced with old dmc firmware.
While debugging PC10 entry issue for skylake found
with latest dmc firmware version 1.18 without byte
swapping dmc is working fine and able to enter PC10.

v1: Initial version.

v2: Corrected firmware size during memcpy(). (Suggested by Sunil)

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/i915_drv.h  |  2 +-
 drivers/gpu/drm/i915/intel_csr.c | 16 ++++------------
 2 files changed, 5 insertions(+), 13 deletions(-)

Comments

Kamath, Sunil Aug. 4, 2015, 11:24 a.m. UTC | #1
On Monday 03 August 2015 09:55 PM, Animesh Manna wrote:
> This patch contains the changes to remove the byte
> swapping logic introduced with old dmc firmware.
> While debugging PC10 entry issue for skylake found
> with latest dmc firmware version 1.18 without byte
> swapping dmc is working fine and able to enter PC10.
>
> v1: Initial version.
>
> v2: Corrected firmware size during memcpy(). (Suggested by Sunil)
>
> 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/i915_drv.h  |  2 +-
>   drivers/gpu/drm/i915/intel_csr.c | 16 ++++------------
>   2 files changed, 5 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b94ada9..9d0ee58 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -742,7 +742,7 @@ enum csr_state {
>   
>   struct intel_csr {
>   	const char *fw_path;
> -	__be32 *dmc_payload;
> +	uint32_t *dmc_payload;
>   	uint32_t dmc_fw_size;
>   	uint32_t mmio_count;
>   	uint32_t mmioaddr[8];
> diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
> index 6d8a7bf..ba1ae03 100644
> --- a/drivers/gpu/drm/i915/intel_csr.c
> +++ b/drivers/gpu/drm/i915/intel_csr.c
> @@ -244,7 +244,7 @@ void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
>   void intel_csr_load_program(struct drm_device *dev)
>   {
>   	struct drm_i915_private *dev_priv = dev->dev_private;
> -	__be32 *payload = dev_priv->csr.dmc_payload;
> +	u32 *payload = dev_priv->csr.dmc_payload;
>   	uint32_t i, fw_size;
>   
>   	if (!IS_GEN9(dev)) {
> @@ -256,7 +256,7 @@ void intel_csr_load_program(struct drm_device *dev)
>   	fw_size = dev_priv->csr.dmc_fw_size;
>   	for (i = 0; i < fw_size; i++)
>   		I915_WRITE(CSR_PROGRAM_BASE + i * 4,
> -			(u32 __force)payload[i]);
> +			payload[i]);
>   
>   	for (i = 0; i < dev_priv->csr.mmio_count; i++) {
>   		I915_WRITE(dev_priv->csr.mmioaddr[i],
> @@ -279,7 +279,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>   	char substepping = intel_get_substepping(dev);
>   	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
>   	uint32_t i;
> -	__be32 *dmc_payload;
> +	uint32_t *dmc_payload;
>   	bool fw_loaded = false;
>   
>   	if (!fw) {
> @@ -375,15 +375,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)
>   	}
>   
>   	dmc_payload = csr->dmc_payload;
> -	for (i = 0; i < dmc_header->fw_size; i++) {
> -		uint32_t *tmp = (u32 *)&fw->data[readcount + i * 4];
> -		/*
> -		 * The firmware payload is an array of 32 bit words stored in
> -		 * little-endian format in the firmware image and programmed
> -		 * as 32 bit big-endian format to memory.
> -		 */
> -		dmc_payload[i] = cpu_to_be32(*tmp);
> -	}
> +	memcpy(dmc_payload, &fw->data[readcount], nbytes);
>   
>   	/* load csr program during system boot, as needed for DC states */
>   	intel_csr_load_program(dev);
Thanks for addressing review comments.
Small change can be done in Gerrit message to generalize the issue to fw 
1.0+. Else fine.

Valid bug fix.

Reviewed-by: A.Sunil Kamath <sunil.kamath@intel.com>
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b94ada9..9d0ee58 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -742,7 +742,7 @@  enum csr_state {
 
 struct intel_csr {
 	const char *fw_path;
-	__be32 *dmc_payload;
+	uint32_t *dmc_payload;
 	uint32_t dmc_fw_size;
 	uint32_t mmio_count;
 	uint32_t mmioaddr[8];
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index 6d8a7bf..ba1ae03 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -244,7 +244,7 @@  void intel_csr_load_status_set(struct drm_i915_private *dev_priv,
 void intel_csr_load_program(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	__be32 *payload = dev_priv->csr.dmc_payload;
+	u32 *payload = dev_priv->csr.dmc_payload;
 	uint32_t i, fw_size;
 
 	if (!IS_GEN9(dev)) {
@@ -256,7 +256,7 @@  void intel_csr_load_program(struct drm_device *dev)
 	fw_size = dev_priv->csr.dmc_fw_size;
 	for (i = 0; i < fw_size; i++)
 		I915_WRITE(CSR_PROGRAM_BASE + i * 4,
-			(u32 __force)payload[i]);
+			payload[i]);
 
 	for (i = 0; i < dev_priv->csr.mmio_count; i++) {
 		I915_WRITE(dev_priv->csr.mmioaddr[i],
@@ -279,7 +279,7 @@  static void finish_csr_load(const struct firmware *fw, void *context)
 	char substepping = intel_get_substepping(dev);
 	uint32_t dmc_offset = CSR_DEFAULT_FW_OFFSET, readcount = 0, nbytes;
 	uint32_t i;
-	__be32 *dmc_payload;
+	uint32_t *dmc_payload;
 	bool fw_loaded = false;
 
 	if (!fw) {
@@ -375,15 +375,7 @@  static void finish_csr_load(const struct firmware *fw, void *context)
 	}
 
 	dmc_payload = csr->dmc_payload;
-	for (i = 0; i < dmc_header->fw_size; i++) {
-		uint32_t *tmp = (u32 *)&fw->data[readcount + i * 4];
-		/*
-		 * The firmware payload is an array of 32 bit words stored in
-		 * little-endian format in the firmware image and programmed
-		 * as 32 bit big-endian format to memory.
-		 */
-		dmc_payload[i] = cpu_to_be32(*tmp);
-	}
+	memcpy(dmc_payload, &fw->data[readcount], nbytes);
 
 	/* load csr program during system boot, as needed for DC states */
 	intel_csr_load_program(dev);