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
