[Intel-gfx] [bug report] drm/i915/guc: Suspend/resume implementation for new interface

[Intel-gfx] [bug report] drm/i915/guc: Suspend/resume implementation for new interface

[Dan Carpenter]

Hello Matthew Brost,

The patch cad46a332f3d: "drm/i915/guc: Suspend/resume implementation
for new interface" from Jul 26, 2021, leads to the following Smatch
static checker warning:

	drivers/gpu/drm/i915/gt/uc/intel_guc.c:655 intel_guc_suspend()
	error: passing non negative 268435455 to ERR_PTR

drivers/gpu/drm/i915/gt/uc/intel_guc.c
    631 int intel_guc_suspend(struct intel_guc *guc)
    632 {
    633         int ret;
    634         u32 action[] = {
    635                 INTEL_GUC_ACTION_CLIENT_SOFT_RESET,
    636         };
    637 
    638         if (!intel_guc_is_ready(guc))
    639                 return 0;
    640 
    641         if (intel_guc_submission_is_used(guc)) {
    642                 /*
    643                  * This H2G MMIO command tears down the GuC in two steps. First it will
    644                  * generate a G2H CTB for every active context indicating a reset. In
    645                  * practice the i915 shouldn't ever get a G2H as suspend should only be
    646                  * called when the GPU is idle. Next, it tears down the CTBs and this
    647                  * H2G MMIO command completes.
    648                  *
    649                  * Don't abort on a failure code from the GuC. Keep going and do the
    650                  * clean up in santize() and re-initialisation on resume and hopefully
    651                  * the error here won't be problematic.
    652                  */
    653                 ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
    654                 if (ret)
--> 655                         guc_err(guc, "suspend: RESET_CLIENT action failed with %pe\n",
    656                                 ERR_PTR(ret));

A positive value of ret doesn't necessarily indicate an error.  The
success path does this:

	ret = FIELD_GET(GUC_HXG_RESPONSE_MSG_0_DATA0, header);

Also I was really hoping someone would make %e print the error string.
Using ERR_PTR() in the printk is an ugly hack around.  (Mind you, I'm
too lazy to lift a finger to help so who am I to talk...)

    657         }
    658 
    659         /* Signal that the GuC isn't running. */
    660         intel_guc_sanitize(guc);
    661 
    662         return 0;
    663 }

regards,
dan carpenter