Re: [Intel-gfx] [PATCH v4 2/8] drm/i915/skl: Add DC5 Trigger Sequence.

[Animesh Manna]

On 4/16/2015 3:18 PM, Imre Deak wrote:

On to, 2015-04-16 at 12:25 +0300, Imre Deak wrote:

On to, 2015-04-16 at 14:22 +0530, Animesh Manna wrote:

  [...]
@@ -223,11 +244,13 @@ static void finish_csr_load(const struct firmware *fw, 
void *context)

if (!fw) {

                i915_firmware_load_error_print(csr->fw_path, 0);
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }

if ((stepping == -ENODATA) || (substepping == -ENODATA)) {

                DRM_ERROR("Unknown stepping info, firmware loading failed\n");
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }

@@ -237,6 +260,7 @@ static void finish_csr_load(const struct firmware *fw, void *context)

                (css_header->header_len * 4)) {
                DRM_ERROR("Firmware has wrong CSS header length %u bytes\n",
                        (css_header->header_len * 4));
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }
        readcount += sizeof(struct intel_css_header);
@@ -248,6 +272,7 @@ static void finish_csr_load(const struct firmware *fw, void 
*context)
                (package_header->header_len * 4)) {
                DRM_ERROR("Firmware has wrong package header length %u bytes\n",
                        (package_header->header_len * 4));
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }
        readcount += sizeof(struct intel_package_header);
@@ -268,6 +293,7 @@ static void finish_csr_load(const struct firmware *fw, void 
*context)
        }
        if (dmc_offset == CSR_DEFAULT_FW_OFFSET) {
                DRM_ERROR("Firmware not supported for %c stepping\n", stepping);
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }
        readcount += dmc_offset;
@@ -277,6 +303,7 @@ static void finish_csr_load(const struct firmware *fw, void 
*context)
        if (sizeof(struct intel_dmc_header) != (dmc_header->header_len)) {
                DRM_ERROR("Firmware has wrong dmc header length %u bytes\n",
                                (dmc_header->header_len));
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }
        readcount += sizeof(struct intel_dmc_header);
@@ -285,6 +312,7 @@ static void finish_csr_load(const struct firmware *fw, void 
*context)
        if (dmc_header->mmio_count > ARRAY_SIZE(csr->mmioaddr)) {
                DRM_ERROR("Firmware has wrong mmio count %u\n",
                                                dmc_header->mmio_count);
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }
        csr->mmio_count = dmc_header->mmio_count;
@@ -293,6 +321,7 @@ static void finish_csr_load(const struct firmware *fw, void 
*context)
                        dmc_header->mmioaddr[i] > CSR_MMIO_END_RANGE) {
                        DRM_ERROR(" Firmware has wrong mmio address 0x%x\n",
                                                dmc_header->mmioaddr[i]);
+                       intel_csr_load_status_set(dev_priv, FW_FAILED);
                        goto out;
                }
                csr->mmioaddr[i] = dmc_header->mmioaddr[i];
@@ -303,6 +332,7 @@ static void finish_csr_load(const struct firmware *fw, void 
*context)
        nbytes = dmc_header->fw_size * 4;
        if (nbytes > CSR_MAX_FW_SIZE) {
                DRM_ERROR("CSR firmware too big (%u) bytes\n", nbytes);
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }
        csr->dmc_fw_size = dmc_header->fw_size;
@@ -310,6 +340,7 @@ static void finish_csr_load(const struct firmware *fw, void 
*context)
        csr->dmc_payload = kmalloc(nbytes, GFP_KERNEL);
        if (!csr->dmc_payload) {
                DRM_ERROR("Memory allocation failed for dmc payload\n");
+               intel_csr_load_status_set(dev_priv, FW_FAILED);
                goto out;
        }

@@ -327,6 +358,11 @@ static void finish_csr_load(const struct firmware *fw, void *context)

        /* load csr program during system boot, as needed for DC states */
        intel_csr_load_program(dev);
  out:

Also, since you have to anyway change the patch: could you replace all
the above intel_csr_load_status_set(dev_priv, FW_FAILED); with one here
using a fw_loaded bool that you set after intel_csr_load_program?

Now we do not have fw_loaded bool in current patch, enum csr_state is used to 
capture the current state.
Want me to add fw_loaded bool and call intel_csr_load_status_set(dev_priv, 
FW_FAILED) based on that?

Regards,
Animesh

+       /*
+        * Release the runtime pm reference obtained when
+        * CSR wasn't loaded.
+        */
+       intel_runtime_pm_put(dev_priv);

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx