| Message ID | 1392723370-4772-2-git-send-email-p.zabel@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 18, 2014 at 12:36:02PM +0100, Philipp Zabel wrote: > From: Lucas Stach <l.stach@pengutronix.de> > > Since imx_drm_encoder_parse_of is called from the encoder bind callbacks, > it is too late to request probe deferral. Rather the core should make sure > that the crtcs are bound before the encoders, after all needed components > are probed. Why is it too late? -EPROBE_DEFER from this point will cause the driver initialisation to correctly unwind and return -EPROBE_DEFER to the last-to-be-added component. > This fixes probe failure when using the LDB on i.MX6. More details please.
Am Montag, den 24.02.2014, 15:49 +0000 schrieb Russell King - ARM Linux: > On Tue, Feb 18, 2014 at 12:36:02PM +0100, Philipp Zabel wrote: > > From: Lucas Stach <l.stach@pengutronix.de> > > > > Since imx_drm_encoder_parse_of is called from the encoder bind callbacks, > > it is too late to request probe deferral. Rather the core should make sure > > that the crtcs are bound before the encoders, after all needed components > > are probed. > > Why is it too late? -EPROBE_DEFER from this point will cause the driver > initialisation to correctly unwind and return -EPROBE_DEFER to the > last-to-be-added component. Hmm, you are right. I have conflated two separate issues here. I'll back that out. > > This fixes probe failure when using the LDB on i.MX6. > > More details please. One issue was that the DT parsing code would try to add the imx-ldb component right after the first crtc, and then its bind would fail in imx_drm_encoder_parse_of because the three remaining crtcs were not yet registered. This is already fixed by adding the crtc components first. The other issue is that once we add bridges that also have output ports, imx_drm_encoder_parse_of needs to skip those. thanks Philipp
Am Montag, den 24.02.2014, 17:56 +0100 schrieb Philipp Zabel: > Am Montag, den 24.02.2014, 15:49 +0000 schrieb Russell King - ARM Linux: > > On Tue, Feb 18, 2014 at 12:36:02PM +0100, Philipp Zabel wrote: > > > From: Lucas Stach <l.stach@pengutronix.de> > > > > > > Since imx_drm_encoder_parse_of is called from the encoder bind callbacks, > > > it is too late to request probe deferral. Rather the core should make sure > > > that the crtcs are bound before the encoders, after all needed components > > > are probed. > > > > Why is it too late? -EPROBE_DEFER from this point will cause the driver > > initialisation to correctly unwind and return -EPROBE_DEFER to the > > last-to-be-added component. > > Hmm, you are right. I have conflated two separate issues here. I'll back > that out. > > > > This fixes probe failure when using the LDB on i.MX6. > > > > More details please. > > One issue was that the DT parsing code would try to add the imx-ldb > component right after the first crtc, and then its bind would fail > in imx_drm_encoder_parse_of because the three remaining crtcs were > not yet registered. This is already fixed by adding the crtc > components first. On second thought, now that the crtcs are all bound before the encoders, we'll never even reach this point until all crtcs are available. regards Philipp
On Mon, Feb 24, 2014 at 05:56:38PM +0100, Philipp Zabel wrote: > Am Montag, den 24.02.2014, 15:49 +0000 schrieb Russell King - ARM Linux: > One issue was that the DT parsing code would try to add the imx-ldb > component right after the first crtc, and then its bind would fail > in imx_drm_encoder_parse_of because the three remaining crtcs were > not yet registered. This is already fixed by adding the crtc > components first. That's what happens anyway - we add /all/ the CRTCs first before we start adding the connectors. That means the CRTCs will be bound before the connectors (ldb). What you're probably missing is that with what's in my branch, if you want both CRTCs or the second CRTC in the IPU pair, you must specify both in the "crtcs" property - iow "crtcs = <&ipuX 0>, <&ipuX 1>". The reason is there's no way to differentiate the individual platform devices there without delving into the platform data.
Am Montag, den 24.02.2014, 17:06 +0000 schrieb Russell King - ARM Linux: > On Mon, Feb 24, 2014 at 05:56:38PM +0100, Philipp Zabel wrote: > > Am Montag, den 24.02.2014, 15:49 +0000 schrieb Russell King - ARM Linux: > > One issue was that the DT parsing code would try to add the imx-ldb > > component right after the first crtc, and then its bind would fail > > in imx_drm_encoder_parse_of because the three remaining crtcs were > > not yet registered. This is already fixed by adding the crtc > > components first. > > That's what happens anyway - we add /all/ the CRTCs first before we > start adding the connectors. Yes, I know. I accidentally removed that feature when I switched to parsing the OF graph instead of the crtcs and encoder properties, but I've fixed this again in v3 without moving this patch out of the way. regards Philipp
diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c index dcba518..98a97a2 100644 --- a/drivers/staging/imx-drm/imx-drm-core.c +++ b/drivers/staging/imx-drm/imx-drm-core.c @@ -457,21 +457,13 @@ int imx_drm_encoder_parse_of(struct drm_device *drm, return ret; id = args.args_count > 0 ? args.args[0] : 0; - mask = imx_drm_find_crtc_mask(imxdrm, args.np, id); + crtc_mask |= imx_drm_find_crtc_mask(imxdrm, args.np, id); of_node_put(args.np); - - /* - * If we failed to find the CRTC(s) which this encoder is - * supposed to be connected to, it's because the CRTC has - * not been registered yet. Defer probing, and hope that - * the required CRTC is added later. - */ - if (mask == 0) - return -EPROBE_DEFER; - - crtc_mask |= mask; } + if (i == 0 || !crtc_mask) + return -ENOENT; + encoder->possible_crtcs = crtc_mask; /* FIXME: this is the mask of outputs which can clone this output. */