| Message ID | 20221130140217.3196414-4-michael.riesch@wolfvision.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/rockchip: vop2: add support for the rgb output block | expand |
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@intel.com>
| Reported-by: Dan Carpenter <error27@gmail.com>
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.
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++) {
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@intel.com> > | Reported-by: Dan Carpenter <error27@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++) { >
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c index 105a548d0abe..94fddbf70ff6 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c @@ -2246,7 +2246,7 @@ static struct vop2_video_port *find_vp_without_primary(struct vop2 *vop2) #define NR_LAYERS 6 -static int vop2_create_crtc(struct vop2 *vop2) +static int vop2_create_crtcs(struct vop2 *vop2) { const struct vop2_data *vop2_data = vop2->data; struct drm_device *drm = vop2->drm; @@ -2371,15 +2371,25 @@ static int vop2_create_crtc(struct vop2 *vop2) return 0; } -static void vop2_destroy_crtc(struct drm_crtc *crtc) +static void vop2_destroy_crtcs(struct vop2 *vop2) { - of_node_put(crtc->port); + struct drm_device *drm = vop2->drm; + struct list_head *crtc_list = &drm->mode_config.crtc_list; + struct list_head *plane_list = &drm->mode_config.plane_list; + struct drm_crtc *crtc, *tmpc; + struct drm_plane *plane, *tmpp; + + list_for_each_entry_safe(plane, tmpp, plane_list, head) + drm_plane_cleanup(plane); /* * Destroy CRTC after vop2_plane_destroy() since vop2_disable_plane() * references the CRTC. */ - drm_crtc_cleanup(crtc); + list_for_each_entry_safe(crtc, tmpc, crtc_list, head) { + of_node_put(crtc->port); + drm_crtc_cleanup(crtc); + } } static struct reg_field vop2_cluster_regs[VOP2_WIN_MAX_REG] = { @@ -2683,7 +2693,7 @@ static int vop2_bind(struct device *dev, struct device *master, void *data) if (ret) return ret; - ret = vop2_create_crtc(vop2); + ret = vop2_create_crtcs(vop2); if (ret) return ret; @@ -2697,19 +2707,10 @@ static int vop2_bind(struct device *dev, struct device *master, void *data) static void vop2_unbind(struct device *dev, struct device *master, void *data) { struct vop2 *vop2 = dev_get_drvdata(dev); - struct drm_device *drm = vop2->drm; - struct list_head *plane_list = &drm->mode_config.plane_list; - struct list_head *crtc_list = &drm->mode_config.crtc_list; - struct drm_crtc *crtc, *tmpc; - struct drm_plane *plane, *tmpp; pm_runtime_disable(dev); - list_for_each_entry_safe(plane, tmpp, plane_list, head) - drm_plane_cleanup(plane); - - list_for_each_entry_safe(crtc, tmpc, crtc_list, head) - vop2_destroy_crtc(crtc); + vop2_destroy_crtcs(vop2); } const struct component_ops vop2_component_ops = {
Let the function name vop2_create_crtcs reflect that the function creates multiple CRTCS. Also, use a symmetric function pair to create and destroy the CRTCs and the corresponding planes. Signed-off-by: Michael Riesch <michael.riesch@wolfvision.net> --- drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 31 ++++++++++---------- 1 file changed, 16 insertions(+), 15 deletions(-)