> On Jul 5, 2017, at 18:56, Hyun Kwon <hyun.k...@xilinx.com> wrote: > > For VDMA, I think you can use the DMA engine complete callback to generate > the vblank. > > But, please note that it's not a global solution, and it doesn't work for > some other pipelines. For example, the ZU+ DPDMA operation is synchronized > with vblank, not with the done event. Thus if the page flip request comes in > between done event and vblank, it'll end up flipping pages while the > application would expect the flip to take place after vblank.
Thanks for that warning. I will make sure I get the timing right for my pipeline. I will probably not submit this work upstream... yet. > > For the xilinx drm clean up, what you described in the previous email, to use > drm_of_component_probe(), makes sense. I'm doing the clean in that direction > too. In the end I did not use drm_of_component_probe() as it make assumptions that the CRTC is a component and treats it specially, failing that, the function aborts. Since I didn't want to refactor xilinx_drm_crtc to a component at this stage, especially knowing the refactoring that is upcoming, I went for a similar but manual way. Here it is, just for fun... /* component master bind callback */ static int xilinx_drm_bind(struct device *dev) { struct drm_device *drm; struct xilinx_drm_private *private; struct drm_encoder *encoder; struct drm_connector *connector; struct device_node *encoder_node; unsigned int bpp, align, i = 0; int ret; private = devm_kzalloc(dev, sizeof(*private), GFP_KERNEL); if (!private) return -ENOMEM; dev_set_drvdata(dev, private); drm = drm_dev_alloc(&xilinx_drm_driver, dev); if (IS_ERR(drm)) return PTR_ERR(drm); drm->dev_private = private; private->drm = drm; platform_set_drvdata(to_platform_device(dev), private); drm_mode_config_init(drm); /* create a xilinx crtc */ private->crtc = xilinx_drm_crtc_create(drm); if (IS_ERR(private->crtc)) { DRM_DEBUG_DRIVER("failed to create xilinx crtc\n"); ret = PTR_ERR(private->crtc); goto err_free; } xilinx_drm_mode_config_init(drm); while ((encoder_node = of_parse_phandle(drm->dev->of_node, "xlnx,encoder-slave", i))) { encoder = xilinx_drm_encoder_create(drm, encoder_node); of_node_put(encoder_node); if (IS_ERR(encoder)) { DRM_DEBUG_DRIVER("failed to create xilinx encoder\n"); ret = PTR_ERR(encoder); goto err_free; } connector = xilinx_drm_connector_create(drm, encoder, i); if (IS_ERR(connector)) { DRM_DEBUG_DRIVER("failed to create xilinx connector\n"); ret = PTR_ERR(connector); goto err_free; } i++; } ret = drm_dev_register(drm, 0); if (ret) { DRM_ERROR("failed to register drm_dev: %d\n", ret); goto err_free; } ret = component_bind_all(dev, drm); if (ret) { DRM_ERROR("Failed to bind all components\n"); goto err_unregister; } ret = drm_vblank_init(drm, 1); if (ret) { dev_err(dev, "failed to initialize vblank\n"); goto err_unbind_all; } /* enable irq to enable vblank feature */ drm->irq_enabled = true; /* initialize xilinx framebuffer */ bpp = xilinx_drm_format_bpp(xilinx_drm_crtc_get_format(private->crtc)); align = xilinx_drm_crtc_get_align(private->crtc); private->fb = xilinx_drm_fb_init(drm, bpp, 1, 1, align, xilinx_drm_fbdev_vres); if (IS_ERR(private->fb)) { DRM_ERROR("failed to initialize drm cma fb\n"); ret = PTR_ERR(private->fb); goto err_vblank_cleanup; } drm_helper_disable_unused_functions(drm); drm_mode_config_reset(drm); drm_kms_helper_poll_init(drm); return 0; err_vblank_cleanup: drm_vblank_cleanup(drm); err_unbind_all: component_unbind_all(dev, drm); err_unregister: drm_dev_unregister(drm); err_free: drm_mode_config_cleanup(drm); dev_set_drvdata(dev, NULL); drm_dev_unref(drm); return ret; } static void xilinx_drm_unbind(struct device *dev) { struct xilinx_drm_private *private = dev_get_drvdata(dev); struct drm_device *drm = private->drm; drm_kms_helper_poll_fini(drm); xilinx_drm_fb_fini(private->fb); component_unbind_all(dev, drm); drm_vblank_cleanup(drm); drm_mode_config_cleanup(drm); drm_dev_unregister(drm); drm_dev_unref(drm); drm->dev_private = NULL; dev_set_drvdata(dev, NULL); } static const struct component_master_ops xilinx_drm_component_master_ops = { .bind = xilinx_drm_bind, .unbind = xilinx_drm_unbind, }; static int compare_dev(struct device *dev, void *data) { return dev->of_node == data; } /* init xilinx drm platform */ static int xilinx_drm_platform_probe(struct platform_device *pdev) { struct device_node *remote_dev, *ep = NULL; struct component_match *match = NULL; while (1) { ep = of_graph_get_next_endpoint(pdev->dev.of_node, ep); if (!ep) break; of_node_put(ep); remote_dev = of_graph_get_remote_port_parent(ep); if (!remote_dev || !of_device_is_available(remote_dev)) { of_node_put(remote_dev); continue; } component_match_add(&pdev->dev, &match, compare_dev, remote_dev); of_node_put(remote_dev); } return component_master_add_with_match(&pdev->dev, &xilinx_drm_component_master_ops, match); } /* exit xilinx drm platform */ static int xilinx_drm_platform_remove(struct platform_device *pdev) { component_master_del(&pdev->dev, &xilinx_drm_component_master_ops); return 0; } There is still a bad pointer dereference at unbind time which doesn't annoy me enough yet since I never unbind, I just shutdown the device. > I can post my local branch to github and point it to you, even though I'm not > sure how much it'd be helpful as it's more like scribble for now. Then I can > also include you when I post the patch to Xilinx mailing list for internal > review, which I expect to happen in July timeframe. You can flag your commit with "WIP: ..." to warn anyone looking, and detail your commit with your concerns and challenges. Onlookers may just chime in to provide insights ;) It's a good timing for this work since I am actively building our support for our display pipeline which is not covered by the current driver's state. If we collaborate, I will want to submit my changes to allow others wanting to integrate LVDS panels in xilinx_drm to do so. Right now, with xilinx_drm using the legacy helpers, my approach requires a small hack in an atomic helper function. Cheers! _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel