[RFC PATCH 2/3] staging: imx-drm-core: Use graph to find connection between crtc and encoder (original) (raw)

Russell King - ARM Linux linux at arm.linux.org.uk
Mon Feb 10 08:26:31 PST 2014

On Mon, Jan 06, 2014 at 03:52:01PM +0100, Philipp Zabel wrote:

@@ -438,24 +453,21 @@ int imxdrmencoderparseof(struct drmdevice *drm, struct drmencoder *encoder, struct devicenode *np) { struct imxdrmdevice *imxdrm = drm->devprivate; + struct devicenode *ep, *lastep = NULL; uint32t crtcmask = 0; int i, ret = 0;

for (i = 0; !ret; i++) { - struct ofphandleargs args; uint32t mask; - int id; - ret = ofparsephandlewithargs(np, "crtcs", "#crtc-cells", i, - &args); - if (ret == -ENOENT) + ep = v4l2ofgetnextendpoint(np, lastep); + if (lastep) + ofnodeput(lastep); + if (!ep) break; - if (ret < 0)_ _- return ret;_ _- id = args.argscount > 0 ? args.args[0] : 0; - mask = imxdrmfindcrtcmask(imxdrm, args.np, id); - ofnodeput(args.np); + /* CSI */ + mask = imxdrmfindcrtcmask(imxdrm, ep); /* * If we failed to find the CRTC(s) which this encoder is @@ -463,12 +475,20 @@ int imxdrmencoderparseof(struct drmdevice *drm, * not been registered yet. Defer probing, and hope that * the required CRTC is added later. */ - if (mask == 0) + if (mask == 0) { + ofnodeput(ep); return -EPROBEDEFER; + } crtcmask |= mask; + lastep = ep; } + if (ep) + ofnodeput(ep);

Why is this loop soo complicated? Why do you need to mess around with this "last_ep" stuff - you don't actually end up using it.

The loop reduces down to this without comments:

 for (i = 0; !ret; i++) {
     uint32_t mask;

    ep = v4l2_of_get_next_endpoint(np, last_ep);
    if (!ep)
        break;

    /* CSI */
    mask = imx_drm_find_crtc_mask(imxdrm, ep);
    of_node_put(ep);

    if (mask == 0)
        return -EPROBE_DEFER;

    crtc_mask |= mask;
}

Now, here's the big question: why do we want to use v4l2_* here? We may want to use this functionality, but if this functionality is going to be used outside of v4l2, it needs to become something generic, not v4l2 specific.

Let's think about this for a moment... if we want to build imx-drm into the kernel, can we do it with modular videodev, or with videodev completely unconfigured. We may wish to do this because we have no videodev requirement on a platform. Should the build fail because the v4l2 function isn't there?

So, before we can change this, I think we first need to get agreement from Mauro to move this function out of V4L2, so that it can be available independently of V4L2.

-- FTTC broadband for 0.8mile line: 5.8Mbps down 500kbps up. Estimation in database were 13.1 to 19Mbit for a good line, about 7.5+ for a bad. Estimate before purchase was "up to 13.2Mbit".

More information about the dri-devel mailing list