> 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

Reply via email to