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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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++) {
> 



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux