[bug report] drm/msm/adreno: Add support for ACD

[Akhil P Oommen]

On 8/7/2025 9:23 PM, Dan Carpenter wrote:
> Hello Akhil P Oommen,
> 
> Commit b733fe7bff8b ("drm/msm/adreno: Add support for ACD") from Apr
> 19, 2025 (linux-next), leads to the following Smatch static checker
> warning:
> 
> 	drivers/gpu/drm/msm/adreno/a6xx_gmu.c:1700 a6xx_gmu_acd_probe()
> 	error: 'opp' dereferencing possible ERR_PTR()
> 
> drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>     1668 static int a6xx_gmu_acd_probe(struct a6xx_gmu *gmu)
>     1669 {
>     1670         struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
>     1671         struct a6xx_hfi_acd_table *cmd = &gmu->acd_table;
>     1672         struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
>     1673         struct msm_gpu *gpu = &adreno_gpu->base;
>     1674         int ret, i, cmd_idx = 0;
>     1675         extern bool disable_acd;
>     1676 
>     1677         /* Skip ACD probe if requested via module param */
>     1678         if (disable_acd) {
>     1679                 DRM_DEV_ERROR(gmu->dev, "Skipping GPU ACD probe\n");
>     1680                 return 0;
>     1681         }
>     1682 
>     1683         cmd->version = 1;
>     1684         cmd->stride = 1;
>     1685         cmd->enable_by_level = 0;
>     1686 
>     1687         /* Skip freq = 0 and parse acd-level for rest of the OPPs */
>     1688         for (i = 1; i < gmu->nr_gpu_freqs; i++) {
>     1689                 struct dev_pm_opp *opp;
>     1690                 struct device_node *np;
>     1691                 unsigned long freq;
>     1692                 u32 val;
>     1693 
>     1694                 freq = gmu->gpu_freqs[i];
>     1695                 opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, freq, true);
>                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> No error checking.

We are passing back a freq which we pulled out from the opp_table a few
lines before this. So it is unlikely that this call would fail.

But it is okay to add a check here if that would make Smatch checker happy.

-Akhil

> 
>     1696                 np = dev_pm_opp_get_of_node(opp);
>     1697 
>     1698                 ret = of_property_read_u32(np, "qcom,opp-acd-level", &val);
>     1699                 of_node_put(np);
> --> 1700                 dev_pm_opp_put(opp);
>     1701                 if (ret == -EINVAL)
>     1702                         continue;
>     1703                 else if (ret) {
>     1704                         DRM_DEV_ERROR(gmu->dev, "Unable to read acd level for freq %lu\n", freq);
>     1705                         return ret;
>     1706                 }
>     1707 
>     1708                 cmd->enable_by_level |= BIT(i);
>     1709                 cmd->data[cmd_idx++] = val;
>     1710         }
>     1711 
>     1712         cmd->num_levels = cmd_idx;
>     1713 
>     1714         /* It is a problem if qmp node is unavailable when ACD is required */
>     1715         if (cmd->enable_by_level && IS_ERR_OR_NULL(gmu->qmp)) {
>     1716                 DRM_DEV_ERROR(gmu->dev, "Unable to send ACD state to AOSS\n");
>     1717                 return -EINVAL;
>     1718         }
>     1719 
>     1720         /* Otherwise, nothing to do if qmp is unavailable */
>     1721         if (IS_ERR_OR_NULL(gmu->qmp))
>     1722                 return 0;
>     1723 
>     1724         /*
>     1725          * Notify AOSS about the ACD state. AOSS is supposed to assume that ACD is disabled on
>     1726          * system reset. So it is harmless if we couldn't notify 'OFF' state
>     1727          */
>     1728         ret = qmp_send(gmu->qmp, "{class: gpu, res: acd, val: %d}", !!cmd->enable_by_level);
>     1729         if (ret && cmd->enable_by_level) {
>     1730                 DRM_DEV_ERROR(gmu->dev, "Failed to send ACD state to AOSS\n");
>     1731                 return ret;
>     1732         }
>     1733 
>     1734         return 0;
>     1735 }
> 
> regards,
> dan carpenter