Re: [DMC_REDESIGN_V2 07/14] drm/i915/gen9: Simplify csr loading failure printing.

From: Daniel Vetter <daniel@ffwll.ch>
To: Marc Herbert <Marc.Herbert@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [DMC_REDESIGN_V2 07/14] drm/i915/gen9: Simplify csr loading failure printing.
Date: Wed, 7 Oct 2015 10:12:36 +0200	[thread overview]
Message-ID: <20151007081236.GA3383@phenom.ffwll.local> (raw)
In-Reply-To: <56142318.2080309@intel.com>

On Tue, Oct 06, 2015 at 12:38:00PM -0700, Marc Herbert wrote:
> On 30/09/15 07:28, Imre Deak wrote:
> >On ke, 2015-08-26 at 16:58 +0530, Animesh Manna wrote:
> >>
> >>-void i915_firmware_load_error_print(const char *fw_path, int err)
> >>-{
> >>-	DRM_ERROR("failed to load firmware %s (%d)\n", fw_path, err);
> >>-
> >>-	/*
> >>-	 * If the reason is not known assume -ENOENT since that's the most
> >>-	 * usual failure mode.
> >>-	 */
> >>-	if (!err)
> >>-		err = -ENOENT;
> >>-
> >>-	if (!(IS_BUILTIN(CONFIG_DRM_I915) && err == -ENOENT))
> >>-		return;
> >>-
> >>-	DRM_ERROR(
> >>-	  "The driver is built-in, so to load the firmware you need to\n"
> >>-	  "include it either in the kernel (see CONFIG_EXTRA_FIRMWARE) or\n"
> >>-	  "in your initrd/initramfs image.\n");
> >>-}
> >>-
> >
> >The point here was to clarify the reason why the loading failed, since
> >that caused quite a confusion. It was a separate function since the same
> >could've been called from the GuC loader too. I think the error message
> >would be still useful.
> 
> Agreed 100%. The code of this function was a bit confusing, however this
> error message has proved very useful "on the field" many times already.
> Please preserve the message.

I dunno really, this is pretty much known by all the other gpu drivers. If
you load things before the firmware is there it'll fail. Only difference
is that with i915 dmc is the first time we require firmware images for
some features.

We should document this in the release notes for the skl/bxt+ stacks, but
this really isn't all that useful in the kernel imo. It's really just how
fw loading works (with drm drivers at least), and at least distros do have
that operational knowledge since years.

Also only printing something for the built-in case seems wrong, since you
can equally screw things up when it's the driver is module. What we should
have instead is just a DRM_INFO or similar that we're disabling a few pm
features because the firmware isn't there.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx