Quoting Anusha Srivatsa (2017-11-02 20:28:10)
> On Wed, Nov 01, 2017 at 01:24:15PM +0000, Chris Wilson wrote:
> > Quoting Michal Wajdeczko (2017-11-01 13:14:33)
> > > On Wed, 01 Nov 2017 01:11:20 +0100, Anusha Srivatsa
> > > > @@ -172,13 +174,18 @@ static int guc_ucode_xfer_dma(struct
> > > > drm_i915_private *dev_priv,
> > > > */
> > > > ret = wait_for(guc_ucode_response(dev_priv, &status), 100);
> > > > + load_time = ktime_ms_delta(ktime_get(), start_load);
> > > > +
> > > > DRM_DEBUG_DRIVER("DMA status 0x%x, GuC status 0x%x\n",
> > > > I915_READ(DMA_CTRL), status);
> > > > if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
> > > > DRM_ERROR("GuC firmware signature verification failed\n");
> > > > ret = -ENOEXEC;
> > > > - }
> > > > + } else if (load_time > 20)
> > > > + DRM_NOTE("GuC load takes more than acceptable threshold\n");
> > >
> > > Please add threshold and actual load time to the message to let user
> > > know that times
> >
> > The more important question is why are we telling the user this at all;
> > a significant but normal condition. What do we expect them to do? It
> > doesn't impair any functionality of the driver, it just took longer than
> > you expected -- which may be simply because the system was doing
> > something else and we slept for longer.
>
> Chris, I am inclining to have this info more for us than the user. It is more of
> a debug print to give us some data. We can see if firmware takes peculiarly
> long time to load. We know its normal to be within 20ms range. So, alert
> if it takes longer than that...
Sure, but the impact is that this is a user facing message, even marked
as a significant message. We are quite capable of parsing debug
messages; even capable of setting up ftrace to extract this timing info
without adding the dmesg in the first place...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
