Re: [PATCH 10/10] drm/i915/uc: Add params for specifying firmware

[Jani Nikula <jani.nikula@linux.intel.com>]

On Wed, 08 Mar 2017, "Srivatsa, Anusha" <anusha.srivatsa@intel.com> wrote:
>>-----Original Message-----
>>From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of
>>Arkadiusz Hiler
>>Sent: Tuesday, March 7, 2017 7:25 AM
>>To: intel-gfx@lists.freedesktop.org
>>Subject: [Intel-gfx] [PATCH 10/10] drm/i915/uc: Add params for specifying
>>firmware
>>
>>`guc_firmware_path` and `huc_firmware_path` module parameters are added.
>>
>>Using the parameter disables version checks and loads desired firmware instead
>>of the default one.
>
> I see that the effort of this patch makes us test with different
> firmware versions and not just the default one. But is it worth
> introducing two new params ? We already have 3 parameters that are guc
> and huc related.

Obviously I'd prefer there were fewer module parameters, but looks like
they multiply like rabbits...

Back when we decided that we only accept one firmware version, there
were complaints about it becoming hard to test various firmware versions
or to bisect the kernel while keeping the firmware constant. This
addresses those issues. If you decide those are non-issues and the patch
is not needed, I'll point whoever complains about the issues to this
discussion.

>>-	if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
>>-	    uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
>>+	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
>>+		DRM_NOTE("Skipping uC firmware version check\n");

Log the version found in the firmware? Or does that happens somewhere
else already?

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx