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 <l...@intel.com>
> | Reported-by: Dan Carpenter <erro...@gmail.com>
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++) {
>
