| Message ID | 1345528711-27801-3-git-send-email-archit@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 2012-08-21 at 11:28 +0530, Archit Taneja wrote: > Create output instances by having an init function in the probes of the platform > device drivers for different interfaces. Create a small function for each > interface to initialize the output entity's fields type and id. > > In the probe of each interface driver, the output entities are created before > the *_probe_pdata() functions intentionally. This is done to ensure that the > output entity is prepared before the panels connected to the output are > registered. We need the output entities to be ready because OMAPDSS will try > to make connections between overlays, managers, outputs and devices during the > panel's probe. You're referring to the recheck_connections stuff? I have a patch that moves that to omapfb side. But of course it doesn't hurt to initialize the output early. We should generally do the initialization in output driver's probe more or less so that we first setup everything related to the output driver, and after that we register the dssdevs. But I think that's what is already done. So, no complaints =). > Signed-off-by: Archit Taneja <archit@ti.com> > --- > drivers/video/omap2/dss/dpi.c | 20 ++++++++++++++++++++ > drivers/video/omap2/dss/dsi.c | 26 ++++++++++++++++++++++++-- > drivers/video/omap2/dss/hdmi.c | 18 ++++++++++++++++++ > drivers/video/omap2/dss/rfbi.c | 19 +++++++++++++++++++ > drivers/video/omap2/dss/sdi.c | 20 ++++++++++++++++++++ > drivers/video/omap2/dss/venc.c | 20 ++++++++++++++++++++ > 6 files changed, 121 insertions(+), 2 deletions(-) > > diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c > index f260343..4eca2e7 100644 > --- a/drivers/video/omap2/dss/dpi.c > +++ b/drivers/video/omap2/dss/dpi.c > @@ -408,10 +408,30 @@ static void __init dpi_probe_pdata(struct platform_device *pdev) > } > } > > +static int __init dpi_init_output(struct platform_device *pdev) > +{ > + struct omap_dss_output *out; > + > + out = dss_create_output(pdev); > + if (!out) > + return -ENOMEM; > + > + out->id = OMAP_DSS_OUTPUT_DPI; > + out->type = OMAP_DISPLAY_TYPE_DPI; > + > + return 0; > +} > + > static int __init omap_dpi_probe(struct platform_device *pdev) > { > + int r; > + > mutex_init(&dpi.lock); > > + r = dpi_init_output(pdev); > + if (r) > + return r; > + > dpi_probe_pdata(pdev); > > return 0; > diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c > index 659b6cd..22e0873 100644 > --- a/drivers/video/omap2/dss/dsi.c > +++ b/drivers/video/omap2/dss/dsi.c > @@ -4903,6 +4903,23 @@ static void __init dsi_probe_pdata(struct platform_device *dsidev) > } > } > > +static int __init dsi_init_output(struct platform_device *dsidev, > + struct dsi_data *dsi) > +{ > + struct omap_dss_output *out; > + > + out = dss_create_output(dsidev); > + if (!out) > + return -ENOMEM; > + > + out->id = dsi->module_id == 0 ? > + OMAP_DSS_OUTPUT_DSI1 : OMAP_DSS_OUTPUT_DSI2; > + > + out->type = OMAP_DISPLAY_TYPE_DSI; > + > + return 0; As I mentioned in the last email, I think this could be something like: struct omap_dss_output *out = &dsi->output; out->pdev = dsidev; out->id = xxx; out->type = yyy; dss_register_output(out); > +} > + > /* DSI1 HW IP initialisation */ > static int __init omap_dsihw_probe(struct platform_device *dsidev) > { > @@ -4997,10 +5014,14 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev) > else > dsi->num_lanes_supported = 3; > > - dsi_probe_pdata(dsidev); > - > dsi_runtime_put(dsidev); > > + r = dsi_init_output(dsidev, dsi); > + if (r) > + goto err_init_output; > + > + dsi_probe_pdata(dsidev); > + Why do you change the sequence here? Isn't it enough to just add the init_output before probe_pdata? Tomi
On Friday 24 August 2012 06:44 PM, Tomi Valkeinen wrote: > On Tue, 2012-08-21 at 11:28 +0530, Archit Taneja wrote: >> Create output instances by having an init function in the probes of the platform >> device drivers for different interfaces. Create a small function for each >> interface to initialize the output entity's fields type and id. >> >> In the probe of each interface driver, the output entities are created before >> the *_probe_pdata() functions intentionally. This is done to ensure that the >> output entity is prepared before the panels connected to the output are >> registered. We need the output entities to be ready because OMAPDSS will try >> to make connections between overlays, managers, outputs and devices during the >> panel's probe. > > You're referring to the recheck_connections stuff? I have a patch that > moves that to omapfb side. But of course it doesn't hurt to initialize > the output early. I've seen that patch. omapfb would need to take care of connecting outputs to displays, and managers to outputs. This is added in recheck_connections done in a patch #9 of the series. The question is whether we want some initial connections made between outputs and displays by DSS, or should that be left completely to omapfb/omapdrm? > > We should generally do the initialization in output driver's probe more > or less so that we first setup everything related to the output driver, > and after that we register the dssdevs. But I think that's what is > already done. > > So, no complaints =). Another thing that comes up with delaying the recheck_connections stuff is that we can't assume that at the point of panel driver's probe, there is an output connected to the display. That makes it a bit tricky to call an output function in the panel's probe, since it isn't connected to any output at all. An example is when we request for a VC in taal_probe. Since the panel isn't connected to any output yet, we can't really call a dsi function to request for the VC. This particular case can be solved by requesting VCs only when we enable the panel(probably makes more sense this way), but there might be other situations which could get tricky to tackle. > >> Signed-off-by: Archit Taneja <archit@ti.com> >> --- >> drivers/video/omap2/dss/dpi.c | 20 ++++++++++++++++++++ >> drivers/video/omap2/dss/dsi.c | 26 ++++++++++++++++++++++++-- >> drivers/video/omap2/dss/hdmi.c | 18 ++++++++++++++++++ >> drivers/video/omap2/dss/rfbi.c | 19 +++++++++++++++++++ >> drivers/video/omap2/dss/sdi.c | 20 ++++++++++++++++++++ >> drivers/video/omap2/dss/venc.c | 20 ++++++++++++++++++++ >> 6 files changed, 121 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c >> index f260343..4eca2e7 100644 >> --- a/drivers/video/omap2/dss/dpi.c >> +++ b/drivers/video/omap2/dss/dpi.c >> @@ -408,10 +408,30 @@ static void __init dpi_probe_pdata(struct platform_device *pdev) >> } >> } >> >> +static int __init dpi_init_output(struct platform_device *pdev) >> +{ >> + struct omap_dss_output *out; >> + >> + out = dss_create_output(pdev); >> + if (!out) >> + return -ENOMEM; >> + >> + out->id = OMAP_DSS_OUTPUT_DPI; >> + out->type = OMAP_DISPLAY_TYPE_DPI; >> + >> + return 0; >> +} >> + >> static int __init omap_dpi_probe(struct platform_device *pdev) >> { >> + int r; >> + >> mutex_init(&dpi.lock); >> >> + r = dpi_init_output(pdev); >> + if (r) >> + return r; >> + >> dpi_probe_pdata(pdev); >> >> return 0; >> diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c >> index 659b6cd..22e0873 100644 >> --- a/drivers/video/omap2/dss/dsi.c >> +++ b/drivers/video/omap2/dss/dsi.c >> @@ -4903,6 +4903,23 @@ static void __init dsi_probe_pdata(struct platform_device *dsidev) >> } >> } >> >> +static int __init dsi_init_output(struct platform_device *dsidev, >> + struct dsi_data *dsi) >> +{ >> + struct omap_dss_output *out; >> + >> + out = dss_create_output(dsidev); >> + if (!out) >> + return -ENOMEM; >> + >> + out->id = dsi->module_id == 0 ? >> + OMAP_DSS_OUTPUT_DSI1 : OMAP_DSS_OUTPUT_DSI2; >> + >> + out->type = OMAP_DISPLAY_TYPE_DSI; >> + >> + return 0; > > As I mentioned in the last email, I think this could be something like: > > struct omap_dss_output *out = &dsi->output; > > out->pdev = dsidev; > out->id = xxx; > out->type = yyy; > dss_register_output(out); > Right, this is much better, will do it this way. > >> +} >> + >> /* DSI1 HW IP initialisation */ >> static int __init omap_dsihw_probe(struct platform_device *dsidev) >> { >> @@ -4997,10 +5014,14 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev) >> else >> dsi->num_lanes_supported = 3; >> >> - dsi_probe_pdata(dsidev); >> - >> dsi_runtime_put(dsidev); >> >> + r = dsi_init_output(dsidev, dsi); >> + if (r) >> + goto err_init_output; >> + >> + dsi_probe_pdata(dsidev); >> + > > Why do you change the sequence here? Isn't it enough to just add the > init_output before probe_pdata? Yes, I think I didn't see the point in keeping the clocks on for dsi_init_output() and dsi_probe_pdata(), so tried to incorporate that in this patch too :), I'll change this back to the old way, it doesn't make sense in moving around pm runtime calls in this series. Archit -- To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2012-08-27 at 11:49 +0530, Archit Taneja wrote: > On Friday 24 August 2012 06:44 PM, Tomi Valkeinen wrote: > > On Tue, 2012-08-21 at 11:28 +0530, Archit Taneja wrote: > >> Create output instances by having an init function in the probes of the platform > >> device drivers for different interfaces. Create a small function for each > >> interface to initialize the output entity's fields type and id. > >> > >> In the probe of each interface driver, the output entities are created before > >> the *_probe_pdata() functions intentionally. This is done to ensure that the > >> output entity is prepared before the panels connected to the output are > >> registered. We need the output entities to be ready because OMAPDSS will try > >> to make connections between overlays, managers, outputs and devices during the > >> panel's probe. > > > > You're referring to the recheck_connections stuff? I have a patch that > > moves that to omapfb side. But of course it doesn't hurt to initialize > > the output early. > > I've seen that patch. omapfb would need to take care of connecting > outputs to displays, and managers to outputs. This is added in > recheck_connections done in a patch #9 of the series. > > The question is whether we want some initial connections made between > outputs and displays by DSS, or should that be left completely to > omapfb/omapdrm? Good question. I don't know. Perhaps we should set initial connections there, as the cases where we have multiple displays per output are quite rare. > > We should generally do the initialization in output driver's probe more > > or less so that we first setup everything related to the output driver, > > and after that we register the dssdevs. But I think that's what is > > already done. > > > > So, no complaints =). > > Another thing that comes up with delaying the recheck_connections stuff > is that we can't assume that at the point of panel driver's probe, there > is an output connected to the display. That makes it a bit tricky to > call an output function in the panel's probe, since it isn't connected > to any output at all. An example is when we request for a VC in > taal_probe. Since the panel isn't connected to any output yet, we can't > really call a dsi function to request for the VC. This particular case > can be solved by requesting VCs only when we enable the panel(probably > makes more sense this way), but there might be other situations which > could get tricky to tackle. Right. Well, as you said, we can easily move the stuff from taal's probe to enable. There shouldn't be any problems to that. However, this problem is part of the bigger problem that I haven't been able to solve properly: how to manage the probe/enable stuff for panels. Everything would be simple and easy if we had just one panel per output, and we could just get and configure everything at probe. But we can have multiple panels per output, of which only one can be used at a time... That's why we currently acquire most of the display resources at enable, instead of probe. Tomi
diff --git a/drivers/video/omap2/dss/dpi.c b/drivers/video/omap2/dss/dpi.c index f260343..4eca2e7 100644 --- a/drivers/video/omap2/dss/dpi.c +++ b/drivers/video/omap2/dss/dpi.c @@ -408,10 +408,30 @@ static void __init dpi_probe_pdata(struct platform_device *pdev) } } +static int __init dpi_init_output(struct platform_device *pdev) +{ + struct omap_dss_output *out; + + out = dss_create_output(pdev); + if (!out) + return -ENOMEM; + + out->id = OMAP_DSS_OUTPUT_DPI; + out->type = OMAP_DISPLAY_TYPE_DPI; + + return 0; +} + static int __init omap_dpi_probe(struct platform_device *pdev) { + int r; + mutex_init(&dpi.lock); + r = dpi_init_output(pdev); + if (r) + return r; + dpi_probe_pdata(pdev); return 0; diff --git a/drivers/video/omap2/dss/dsi.c b/drivers/video/omap2/dss/dsi.c index 659b6cd..22e0873 100644 --- a/drivers/video/omap2/dss/dsi.c +++ b/drivers/video/omap2/dss/dsi.c @@ -4903,6 +4903,23 @@ static void __init dsi_probe_pdata(struct platform_device *dsidev) } } +static int __init dsi_init_output(struct platform_device *dsidev, + struct dsi_data *dsi) +{ + struct omap_dss_output *out; + + out = dss_create_output(dsidev); + if (!out) + return -ENOMEM; + + out->id = dsi->module_id == 0 ? + OMAP_DSS_OUTPUT_DSI1 : OMAP_DSS_OUTPUT_DSI2; + + out->type = OMAP_DISPLAY_TYPE_DSI; + + return 0; +} + /* DSI1 HW IP initialisation */ static int __init omap_dsihw_probe(struct platform_device *dsidev) { @@ -4997,10 +5014,14 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev) else dsi->num_lanes_supported = 3; - dsi_probe_pdata(dsidev); - dsi_runtime_put(dsidev); + r = dsi_init_output(dsidev, dsi); + if (r) + goto err_init_output; + + dsi_probe_pdata(dsidev); + if (dsi->module_id == 0) dss_debugfs_create_file("dsi1_regs", dsi1_dump_regs); else if (dsi->module_id == 1) @@ -5014,6 +5035,7 @@ static int __init omap_dsihw_probe(struct platform_device *dsidev) #endif return 0; +err_init_output: err_runtime_get: pm_runtime_disable(&dsidev->dev); dsi_put_clocks(dsidev); diff --git a/drivers/video/omap2/dss/hdmi.c b/drivers/video/omap2/dss/hdmi.c index 0cdf246..d32dbc3 100644 --- a/drivers/video/omap2/dss/hdmi.c +++ b/drivers/video/omap2/dss/hdmi.c @@ -890,6 +890,20 @@ static void __init hdmi_probe_pdata(struct platform_device *pdev) } } +static int __init hdmi_init_output(struct platform_device *pdev) +{ + struct omap_dss_output *out; + + out = dss_create_output(pdev); + if (!out) + return -ENOMEM; + + out->id = OMAP_DSS_OUTPUT_HDMI; + out->type = OMAP_DISPLAY_TYPE_HDMI; + + return 0; +} + /* HDMI HW IP initialisation */ static int __init omapdss_hdmihw_probe(struct platform_device *pdev) { @@ -933,6 +947,10 @@ static int __init omapdss_hdmihw_probe(struct platform_device *pdev) dss_debugfs_create_file("hdmi", hdmi_dump_regs); + r = hdmi_init_output(pdev); + if (r) + return r; + hdmi_probe_pdata(pdev); return 0; diff --git a/drivers/video/omap2/dss/rfbi.c b/drivers/video/omap2/dss/rfbi.c index 5a9c0e9..9d9244c 100644 --- a/drivers/video/omap2/dss/rfbi.c +++ b/drivers/video/omap2/dss/rfbi.c @@ -967,6 +967,20 @@ static void __init rfbi_probe_pdata(struct platform_device *pdev) } } +static int __init rfbi_init_output(struct platform_device *pdev) +{ + struct omap_dss_output *out; + + out = dss_create_output(pdev); + if (!out) + return -ENOMEM; + + out->id = OMAP_DSS_OUTPUT_DBI; + out->type = OMAP_DISPLAY_TYPE_DBI; + + return 0; +} + /* RFBI HW IP initialisation */ static int __init omap_rfbihw_probe(struct platform_device *pdev) { @@ -1018,10 +1032,15 @@ static int __init omap_rfbihw_probe(struct platform_device *pdev) dss_debugfs_create_file("rfbi", rfbi_dump_regs); + r = rfbi_init_output(pdev); + if (r) + goto err_init_output; + rfbi_probe_pdata(pdev); return 0; +err_init_output: err_runtime_get: pm_runtime_disable(&pdev->dev); return r; diff --git a/drivers/video/omap2/dss/sdi.c b/drivers/video/omap2/dss/sdi.c index 3bf1bfe..a5632d0 100644 --- a/drivers/video/omap2/dss/sdi.c +++ b/drivers/video/omap2/dss/sdi.c @@ -216,8 +216,28 @@ static void __init sdi_probe_pdata(struct platform_device *pdev) } } +static int __init sdi_init_output(struct platform_device *pdev) +{ + struct omap_dss_output *out; + + out = dss_create_output(pdev); + if (!out) + return -ENOMEM; + + out->id = OMAP_DSS_OUTPUT_SDI; + out->type = OMAP_DISPLAY_TYPE_SDI; + + return 0; +} + static int __init omap_sdi_probe(struct platform_device *pdev) { + int r; + + r = sdi_init_output(pdev); + if (r) + return r; + sdi_probe_pdata(pdev); return 0; diff --git a/drivers/video/omap2/dss/venc.c b/drivers/video/omap2/dss/venc.c index 7d3eef8..264c5ec 100644 --- a/drivers/video/omap2/dss/venc.c +++ b/drivers/video/omap2/dss/venc.c @@ -778,6 +778,20 @@ static void __init venc_probe_pdata(struct platform_device *pdev) } } +static int __init venc_init_output(struct platform_device *pdev) +{ + struct omap_dss_output *out; + + out = dss_create_output(pdev); + if (!out) + return -ENOMEM; + + out->id = OMAP_DSS_OUTPUT_VENC; + out->type = OMAP_DISPLAY_TYPE_VENC; + + return 0; +} + /* VENC HW IP initialisation */ static int __init omap_venchw_probe(struct platform_device *pdev) { @@ -825,10 +839,16 @@ static int __init omap_venchw_probe(struct platform_device *pdev) dss_debugfs_create_file("venc", venc_dump_regs); + r = venc_init_output(pdev); + if (r) + goto err_init_output; + venc_probe_pdata(pdev); return 0; +err_init_output: + venc_panel_exit(); err_panel_init: err_runtime_get: pm_runtime_disable(&pdev->dev);
Create output instances by having an init function in the probes of the platform device drivers for different interfaces. Create a small function for each interface to initialize the output entity's fields type and id. In the probe of each interface driver, the output entities are created before the *_probe_pdata() functions intentionally. This is done to ensure that the output entity is prepared before the panels connected to the output are registered. We need the output entities to be ready because OMAPDSS will try to make connections between overlays, managers, outputs and devices during the panel's probe. Signed-off-by: Archit Taneja <archit@ti.com> --- drivers/video/omap2/dss/dpi.c | 20 ++++++++++++++++++++ drivers/video/omap2/dss/dsi.c | 26 ++++++++++++++++++++++++-- drivers/video/omap2/dss/hdmi.c | 18 ++++++++++++++++++ drivers/video/omap2/dss/rfbi.c | 19 +++++++++++++++++++ drivers/video/omap2/dss/sdi.c | 20 ++++++++++++++++++++ drivers/video/omap2/dss/venc.c | 20 ++++++++++++++++++++ 6 files changed, 121 insertions(+), 2 deletions(-)