Re: [PATCH 3/5] drm/rockchip: vop2: use symmetric function pair vop2_{create,destroy}_crtcs

From: Michael Riesch
Date: Thu Jan 19 2023 - 04:41:58 EST

Next message: Kevin Brodsky: "[PATCH] uapi/linux/const.h: Prefer ISO-friendly __typeof__"
Previous message: Daniel Vacek: "Re: [Intel-wired-lan] [PATCH] ice/ptp: fix the PTP worker retrying indefinitely if the link went down"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Dan,

On 1/3/23 09:07, Dan Carpenter wrote:
> Hi Michael,
>
> url: https://github.com/intel-lab-lkp/linux/commits/Michael-Riesch/drm-rockchip-vop2-add-support-for-the-rgb-output-block/20221130-220346
> base: b7b275e60bcd5f89771e865a8239325f86d9927d
> patch link: https://lore.kernel.org/r/20221130140217.3196414-4-michael.riesch%40wolfvision.net
> patch subject: [PATCH 3/5] drm/rockchip: vop2: use symmetric function pair vop2_{create,destroy}_crtcs
> config: parisc-randconfig-m031-20221225
> compiler: hppa-linux-gcc (GCC) 12.1.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@xxxxxxxxx>
> | Reported-by: Dan Carpenter <error27@xxxxxxxxx>

Thanks for the report.

> New smatch warnings:
> drivers/gpu/drm/rockchip/rockchip_drm_vop2.c:2330 vop2_create_crtcs() error: uninitialized symbol 'possible_crtcs'.
>
> vim +/possible_crtcs +2330 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>
> fb83276e59f2d6 Michael Riesch 2022-11-30 2249 static int vop2_create_crtcs(struct vop2 *vop2)
> 604be85547ce4d Andy Yan 2022-04-22 2250 {
> 604be85547ce4d Andy Yan 2022-04-22 2251 const struct vop2_data *vop2_data = vop2->data;
> 604be85547ce4d Andy Yan 2022-04-22 2252 struct drm_device *drm = vop2->drm;
> 604be85547ce4d Andy Yan 2022-04-22 2253 struct device *dev = vop2->dev;
> 604be85547ce4d Andy Yan 2022-04-22 2254 struct drm_plane *plane;
> 604be85547ce4d Andy Yan 2022-04-22 2255 struct device_node *port;
> 604be85547ce4d Andy Yan 2022-04-22 2256 struct vop2_video_port *vp;
> 604be85547ce4d Andy Yan 2022-04-22 2257 int i, nvp, nvps = 0;
> 604be85547ce4d Andy Yan 2022-04-22 2258 int ret;
> 604be85547ce4d Andy Yan 2022-04-22 2259
> 604be85547ce4d Andy Yan 2022-04-22 2260 for (i = 0; i < vop2_data->nr_vps; i++) {
> 604be85547ce4d Andy Yan 2022-04-22 2261 const struct vop2_video_port_data *vp_data;
> 604be85547ce4d Andy Yan 2022-04-22 2262 struct device_node *np;
> 604be85547ce4d Andy Yan 2022-04-22 2263 char dclk_name[9];
> 604be85547ce4d Andy Yan 2022-04-22 2264
> 604be85547ce4d Andy Yan 2022-04-22 2265 vp_data = &vop2_data->vp[i];
> 604be85547ce4d Andy Yan 2022-04-22 2266 vp = &vop2->vps[i];
> 604be85547ce4d Andy Yan 2022-04-22 2267 vp->vop2 = vop2;
> 604be85547ce4d Andy Yan 2022-04-22 2268 vp->id = vp_data->id;
> 604be85547ce4d Andy Yan 2022-04-22 2269 vp->regs = vp_data->regs;
> 604be85547ce4d Andy Yan 2022-04-22 2270 vp->data = vp_data;
> 604be85547ce4d Andy Yan 2022-04-22 2271
> 604be85547ce4d Andy Yan 2022-04-22 2272 snprintf(dclk_name, sizeof(dclk_name), "dclk_vp%d", vp->id);
> 604be85547ce4d Andy Yan 2022-04-22 2273 vp->dclk = devm_clk_get(vop2->dev, dclk_name);
> 604be85547ce4d Andy Yan 2022-04-22 2274 if (IS_ERR(vp->dclk)) {
> 604be85547ce4d Andy Yan 2022-04-22 2275 drm_err(vop2->drm, "failed to get %s\n", dclk_name);
> 604be85547ce4d Andy Yan 2022-04-22 2276 return PTR_ERR(vp->dclk);
> 604be85547ce4d Andy Yan 2022-04-22 2277 }
> 604be85547ce4d Andy Yan 2022-04-22 2278
> 604be85547ce4d Andy Yan 2022-04-22 2279 np = of_graph_get_remote_node(dev->of_node, i, -1);
> 604be85547ce4d Andy Yan 2022-04-22 2280 if (!np) {
> 604be85547ce4d Andy Yan 2022-04-22 2281 drm_dbg(vop2->drm, "%s: No remote for vp%d\n", __func__, i);
> 604be85547ce4d Andy Yan 2022-04-22 2282 continue;
> 604be85547ce4d Andy Yan 2022-04-22 2283 }
> 604be85547ce4d Andy Yan 2022-04-22 2284 of_node_put(np);
> 604be85547ce4d Andy Yan 2022-04-22 2285
> 604be85547ce4d Andy Yan 2022-04-22 2286 port = of_graph_get_port_by_id(dev->of_node, i);
> 604be85547ce4d Andy Yan 2022-04-22 2287 if (!port) {
> 604be85547ce4d Andy Yan 2022-04-22 2288 drm_err(vop2->drm, "no port node found for video_port%d\n", i);
> 604be85547ce4d Andy Yan 2022-04-22 2289 return -ENOENT;
> 604be85547ce4d Andy Yan 2022-04-22 2290 }
> 604be85547ce4d Andy Yan 2022-04-22 2291
> 604be85547ce4d Andy Yan 2022-04-22 2292 vp->crtc.port = port;
> 604be85547ce4d Andy Yan 2022-04-22 2293 nvps++;
> 604be85547ce4d Andy Yan 2022-04-22 2294 }
> 604be85547ce4d Andy Yan 2022-04-22 2295
> 604be85547ce4d Andy Yan 2022-04-22 2296 nvp = 0;
> 604be85547ce4d Andy Yan 2022-04-22 2297 for (i = 0; i < vop2->registered_num_wins; i++) {
> 604be85547ce4d Andy Yan 2022-04-22 2298 struct vop2_win *win = &vop2->win[i];
> 604be85547ce4d Andy Yan 2022-04-22 2299 u32 possible_crtcs;
> 604be85547ce4d Andy Yan 2022-04-22 2300
> 604be85547ce4d Andy Yan 2022-04-22 2301 if (vop2->data->soc_id == 3566) {
> 604be85547ce4d Andy Yan 2022-04-22 2302 /*
> 604be85547ce4d Andy Yan 2022-04-22 2303 * On RK3566 these windows don't have an independent
> 604be85547ce4d Andy Yan 2022-04-22 2304 * framebuffer. They share the framebuffer with smart0,
> 604be85547ce4d Andy Yan 2022-04-22 2305 * esmart0 and cluster0 respectively.
> 604be85547ce4d Andy Yan 2022-04-22 2306 */
> 604be85547ce4d Andy Yan 2022-04-22 2307 switch (win->data->phys_id) {
> 604be85547ce4d Andy Yan 2022-04-22 2308 case ROCKCHIP_VOP2_SMART1:
> 604be85547ce4d Andy Yan 2022-04-22 2309 case ROCKCHIP_VOP2_ESMART1:
> 604be85547ce4d Andy Yan 2022-04-22 2310 case ROCKCHIP_VOP2_CLUSTER1:
> 604be85547ce4d Andy Yan 2022-04-22 2311 continue;
> 604be85547ce4d Andy Yan 2022-04-22 2312 }
> 604be85547ce4d Andy Yan 2022-04-22 2313 }
> 604be85547ce4d Andy Yan 2022-04-22 2314
> 604be85547ce4d Andy Yan 2022-04-22 2315 if (win->type == DRM_PLANE_TYPE_PRIMARY) {
> 604be85547ce4d Andy Yan 2022-04-22 2316 vp = find_vp_without_primary(vop2);
> 604be85547ce4d Andy Yan 2022-04-22 2317 if (vp) {
> 604be85547ce4d Andy Yan 2022-04-22 2318 possible_crtcs = BIT(nvp);
> 604be85547ce4d Andy Yan 2022-04-22 2319 vp->primary_plane = win;
> 604be85547ce4d Andy Yan 2022-04-22 2320 nvp++;
> 604be85547ce4d Andy Yan 2022-04-22 2321 } else {
> 604be85547ce4d Andy Yan 2022-04-22 2322 /* change the unused primary window to overlay window */
> 604be85547ce4d Andy Yan 2022-04-22 2323 win->type = DRM_PLANE_TYPE_OVERLAY;
> 604be85547ce4d Andy Yan 2022-04-22 2324 }
> 604be85547ce4d Andy Yan 2022-04-22 2325 }
> 604be85547ce4d Andy Yan 2022-04-22 2326
> 604be85547ce4d Andy Yan 2022-04-22 2327 if (win->type == DRM_PLANE_TYPE_OVERLAY)
> 604be85547ce4d Andy Yan 2022-04-22 2328 possible_crtcs = (1 << nvps) - 1;
>
> What about if win->type is not equal to either DRM_PLANE_TYPE_PRIMARY or
> DRM_PLANE_TYPE_OVERLAY? That's what the checker is worried about.

If I am not mistaken this issue is present in mainline and my series
does neither introduce nor fix it. I can add a patch that sorts the
issue out to the v2 of my series, though.

As of now the driver only supports PRIMARY and OVERLAY planes, therefore
the current state is safe to use. But maybe CURSOR planes are supported
in the future.

Rewriting the two if's into a if/else structure and setting the
possible_crtcs variable to zero in the else case should do the trick. If
there aren't any objections to that approach, I'll spin a patch.

Best regards,
Michael

>
> 604be85547ce4d Andy Yan 2022-04-22 2329
> 604be85547ce4d Andy Yan 2022-04-22 @2330 ret = vop2_plane_init(vop2, win, possible_crtcs);
> ^^^^^^^^^^^^^^
>
> 604be85547ce4d Andy Yan 2022-04-22 2331 if (ret) {
> 604be85547ce4d Andy Yan 2022-04-22 2332 drm_err(vop2->drm, "failed to init plane %s: %d\n",
> 604be85547ce4d Andy Yan 2022-04-22 2333 win->data->name, ret);
> 604be85547ce4d Andy Yan 2022-04-22 2334 return ret;
> 604be85547ce4d Andy Yan 2022-04-22 2335 }
> 604be85547ce4d Andy Yan 2022-04-22 2336 }
> 604be85547ce4d Andy Yan 2022-04-22 2337
> 604be85547ce4d Andy Yan 2022-04-22 2338 for (i = 0; i < vop2_data->nr_vps; i++) {
>

Next message: Kevin Brodsky: "[PATCH] uapi/linux/const.h: Prefer ISO-friendly __typeof__"
Previous message: Daniel Vacek: "Re: [Intel-wired-lan] [PATCH] ice/ptp: fix the PTP worker retrying indefinitely if the link went down"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]