Message ID | 1347445874-10779-1-git-send-email-s.hauer@pengutronix.de |
---|---|
State | New |
Headers | show |
On Wed, Sep 12, 2012 at 12:31:09PM +0200, Sascha Hauer wrote: > This patch adds the i.MX glue stuff between i.MX and drm. > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/staging/Kconfig | 2 + > drivers/staging/Makefile | 1 + The patch fails at this point when applying it. What tree did you make it against? Please do so against linux-next or hopefully, the staging-next branch, and resubmit these. Also, will this cause build errors if I don't have the needed patchs that are in the DRM tree in my tree? thanks, greg k-h
On Wed, Sep 12, 2012 at 09:49:29AM -0700, Greg Kroah-Hartman wrote: > On Wed, Sep 12, 2012 at 12:31:09PM +0200, Sascha Hauer wrote: > > This patch adds the i.MX glue stuff between i.MX and drm. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/staging/Kconfig | 2 + > > drivers/staging/Makefile | 1 + > > The patch fails at this point when applying it. > > What tree did you make it against? Please do so against linux-next or > hopefully, the staging-next branch, and resubmit these. > > Also, will this cause build errors if I don't have the needed patchs > that are in the DRM tree in my tree? Yes, the driver won't build without the patches. I'll rebase the IPU support on linux-next once the dependency patches are in place. Thanks Sascha
On 12.09.2012 12:31, Sascha Hauer wrote: > The IPU is the Image Processing Unit found on i.MX51/53/6 SoCs. It > features several units for image processing, this patch adds support > for the units needed for Framebuffer support, namely: > > - Display Controller (dc) > - Display Interface (di) > - Display Multi Fifo Controller (dmfc) > - Display Processor (dp) > - Image DMA Controller (idmac) > > This patch is based on the Freescale driver, but follows a different > approach. The Freescale code implements logical idmac channels and > the handling of the subunits is hidden in common idmac code pathes > in big switch/case statements. This patch instead just provides code > and resource management for the different subunits. The user, in this > case the framebuffer driver, decides how the different units play > together. > > The IPU has other units missing in this patch: > > - CMOS Sensor Interface (csi) > - Video Deinterlacer (vdi) > - Sensor Multi FIFO Controler (smfc) > - Image Converter (ic) > - Image Rotator (irt) > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> .... > +static int ipu_reset(struct ipu_soc *ipu) > +{ > + unsigned long timeout; > + > + ipu_cm_write(ipu, 0x807FFFFF, IPU_MEM_RST); > + > + timeout = jiffies + msecs_to_jiffies(1000); > + while (ipu_cm_read(ipu, IPU_MEM_RST) & 0x80000000) { > + if (time_after(jiffies, timeout)) > + return -ETIME; > + cpu_relax(); > + } > + > + mdelay(300); ^^^^^^^^^^^^ > + return 0; > +} While doing some boot time measurement with i.MX6, we found that the above mdelay(300) is hurting regarding boot time. On i.MX6 you have two IPU instances, so in the end you get 600ms additional delay. Looking at the Freescale code, this function looks like static int ipu_reset(struct ipu_soc *ipu) { int timeout = 1000; ipu_cm_write(ipu, 0x807FFFFF, IPU_MEM_RST); while (ipu_cm_read(ipu, IPU_MEM_RST) & 0x80000000) { if (!timeout--) return -ETIME; msleep(1); } return 0; } So there is a msleep() in the loop but no mdelay() outside. Any idea why the mdelay() is needed here? Or what could be done regarding boot time with this? Note: This is just a question, this shouldn't block the staging process. Best regards Dirk
On Fri, Sep 14, 2012 at 11:29:06AM +0200, Dirk Behme wrote: > On 12.09.2012 12:31, Sascha Hauer wrote: > >+ > >+ timeout = jiffies + msecs_to_jiffies(1000); > >+ while (ipu_cm_read(ipu, IPU_MEM_RST) & 0x80000000) { > >+ if (time_after(jiffies, timeout)) > >+ return -ETIME; > >+ cpu_relax(); > >+ } > >+ > >+ mdelay(300); > ^^^^^^^^^^^^ > > >+ return 0; > >+} > > While doing some boot time measurement with i.MX6, we found that the > above mdelay(300) is hurting regarding boot time. On i.MX6 you have > two IPU instances, so in the end you get 600ms additional delay. > > Looking at the Freescale code, this function looks like > > static int ipu_reset(struct ipu_soc *ipu) > { > int timeout = 1000; > > ipu_cm_write(ipu, 0x807FFFFF, IPU_MEM_RST); > > while (ipu_cm_read(ipu, IPU_MEM_RST) & 0x80000000) { > if (!timeout--) > return -ETIME; > msleep(1); > } > return 0; > } > > So there is a msleep() in the loop but no mdelay() outside. Any idea > why the mdelay() is needed here? Or what could be done regarding > boot time with this? I remember we had issues on i.MX51 or 53 without it, but I would have to recheck it. In any way, I think this should be reworked. The reset takes quite a long time and it's not nice to block the boot process for so long. Some asynchronous reset would be nice here. Sascha
Hi Sascha, On 09/12/2012 03:31 AM, Sascha Hauer wrote: > From: Philipp Zabel<p.zabel@pengutronix.de> > > Signed-off-by: Philipp Zabel<p.zabel@pengutronix.de> > Signed-off-by: Sascha Hauer<s.hauer@pengutronix.de> > --- > .../bindings/staging/imx-drm/fsl-imx-drm.txt | 41 ++++++++++++++++++++ > 1 file changed, 41 insertions(+) > create mode 100644 Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt > > diff --git a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt > new file mode 100644 > index 0000000..07654f0 > --- /dev/null > +++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt > @@ -0,0 +1,41 @@ > +Freescale i.MX IPUv3 > +==================== > + > +Required properties: > +- compatible: Should be "fsl,<chip>-ipu" > +- reg: should be register base and length as documented in the > + datasheet > +- interrupts: Should contain sync interrupt and error interrupt, > + in this order. > +- #crtc-cells: 1, See below > + > +example: > + > +ipu: ipu@18000000 { > + #crtc-cells =<1>; > + compatible = "fsl,imx53-ipu"; > + reg =<0x18000000 0x080000000>; > + interrupts =<11 10>; > +}; > + > +Parallel display support > +======================== > + > +Required properties: > +- compatible: Should be "fsl,imx-parallel-display" > +- crtc: the crtc this display is connected to, see below > +Optional properties: > +- interface_pix_fmt: How this display is connected to the > + crtc. Currently supported types: "rgb24", "rgb565" > +- edid: verbatim EDID data block describing attached display. > +- ddc: phandle describing the i2c bus handling the display data > + channel > + > +example: > + > +display@di0 { > + compatible = "fsl,imx-parallel-display"; > + edid = [edid-data]; > + crtc =<&ipu 0>; > + interface-pix-fmt = "rgb24"; > +}; Do you have a working sample of [edid-data] for a parallel/LVDS/HDMI display or know a good way to produce one? Thanks in advance, Eric
On Wed, Sep 12, 2012 at 12:31:14PM +0200, Sascha Hauer wrote: > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > --- > drivers/staging/imx-drm/TODO | 22 ++++++++++++++++++++++ > 1 file changed, 22 insertions(+) > create mode 100644 drivers/staging/imx-drm/TODO > > diff --git a/drivers/staging/imx-drm/TODO b/drivers/staging/imx-drm/TODO > new file mode 100644 > index 0000000..2075646 > --- /dev/null > +++ b/drivers/staging/imx-drm/TODO > @@ -0,0 +1,22 @@ > +TODO: > +- get DRM Maintainer review for this code > +- Factor out more code to common helper functions > +- decide where to put the base driver. It is not specific to a subsystem > + and would be used by DRM/KMS and media/V4L2 > +- convert irq driver to irq_domain_add_linear > + > +Missing features (not necessarily for moving out out staging): out of? > + > +- Add KMS plane support for CRTC driver > +- Add LDB (LVDS Display Bridge) support > +- Add i.MX6 HDMI support > +- Add support for IC (Image converter) > +- Add support for CSI (CMOS Sensor interface) > +- Add support for VDIC (Video Deinterlacer) > + > +Many work-in-progress patches for the above features exist. Contact > +Sascha Hauer <kernel@pengutronix.de> if you are interested in working > +on a specific feature. > + > +Please send any patches to Greg Kroah-Hartman <greg@kroah.com> and Greg Kroah-Hartman <gregkh@linuxfoundation.org>? > +Sascha Hauer <kernel@pengutronix.de> > -- > 1.7.10.4 > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Sep 18, 2012 at 03:06:36PM -0700, Eric Nelson wrote: > Hi Sascha, > > On 09/12/2012 03:31 AM, Sascha Hauer wrote: > >From: Philipp Zabel<p.zabel@pengutronix.de> > > > >Signed-off-by: Philipp Zabel<p.zabel@pengutronix.de> > >Signed-off-by: Sascha Hauer<s.hauer@pengutronix.de> > >--- > > .../bindings/staging/imx-drm/fsl-imx-drm.txt | 41 ++++++++++++++++++++ > > 1 file changed, 41 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt > > > >+ > >+example: > >+ > >+display@di0 { > >+ compatible = "fsl,imx-parallel-display"; > >+ edid = [edid-data]; > >+ crtc =<&ipu 0>; > >+ interface-pix-fmt = "rgb24"; > >+}; > > Do you have a working sample of [edid-data] for a parallel/LVDS/HDMI > display or know a good way to produce one? No, we are using the of videomode helper, see http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg18618.html It is not included here to not add a dependency on it. We are trying to mainline this separately. Sascha
On Wed, Sep 19, 2012 at 01:53:25PM +0800, Shawn Guo wrote: > On Wed, Sep 12, 2012 at 12:31:14PM +0200, Sascha Hauer wrote: > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > drivers/staging/imx-drm/TODO | 22 ++++++++++++++++++++++ > > 1 file changed, 22 insertions(+) > > create mode 100644 drivers/staging/imx-drm/TODO > > > > diff --git a/drivers/staging/imx-drm/TODO b/drivers/staging/imx-drm/TODO > > new file mode 100644 > > index 0000000..2075646 > > --- /dev/null > > +++ b/drivers/staging/imx-drm/TODO > > @@ -0,0 +1,22 @@ > > +TODO: > > +- get DRM Maintainer review for this code > > +- Factor out more code to common helper functions > > +- decide where to put the base driver. It is not specific to a subsystem > > + and would be used by DRM/KMS and media/V4L2 > > +- convert irq driver to irq_domain_add_linear > > + > > +Missing features (not necessarily for moving out out staging): > > out of? Will fix. > > > + > > +- Add KMS plane support for CRTC driver > > +- Add LDB (LVDS Display Bridge) support > > +- Add i.MX6 HDMI support > > +- Add support for IC (Image converter) > > +- Add support for CSI (CMOS Sensor interface) > > +- Add support for VDIC (Video Deinterlacer) > > + > > +Many work-in-progress patches for the above features exist. Contact > > +Sascha Hauer <kernel@pengutronix.de> if you are interested in working > > +on a specific feature. > > + > > +Please send any patches to Greg Kroah-Hartman <greg@kroah.com> and > > Greg Kroah-Hartman <gregkh@linuxfoundation.org>? Hm, right. I copied this from another TODO file. # find drivers/staging/ -name TODO | xargs grep greg@kroah.com | wc -l 20 Maybe Gregs address should be dropped here, he will show up in get-maintainers.pl anyway. Greg? Sascha
On Wed, Sep 19, 2012 at 09:18:03AM +0200, Sascha Hauer wrote: > On Wed, Sep 19, 2012 at 01:53:25PM +0800, Shawn Guo wrote: > > On Wed, Sep 12, 2012 at 12:31:14PM +0200, Sascha Hauer wrote: > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > --- > > > drivers/staging/imx-drm/TODO | 22 ++++++++++++++++++++++ > > > 1 file changed, 22 insertions(+) > > > create mode 100644 drivers/staging/imx-drm/TODO > > > > > > diff --git a/drivers/staging/imx-drm/TODO b/drivers/staging/imx-drm/TODO > > > new file mode 100644 > > > index 0000000..2075646 > > > --- /dev/null > > > +++ b/drivers/staging/imx-drm/TODO > > > @@ -0,0 +1,22 @@ > > > +TODO: > > > +- get DRM Maintainer review for this code > > > +- Factor out more code to common helper functions > > > +- decide where to put the base driver. It is not specific to a subsystem > > > + and would be used by DRM/KMS and media/V4L2 > > > +- convert irq driver to irq_domain_add_linear > > > + > > > +Missing features (not necessarily for moving out out staging): > > > > out of? > > Will fix. > > > > > > + > > > +- Add KMS plane support for CRTC driver > > > +- Add LDB (LVDS Display Bridge) support > > > +- Add i.MX6 HDMI support > > > +- Add support for IC (Image converter) > > > +- Add support for CSI (CMOS Sensor interface) > > > +- Add support for VDIC (Video Deinterlacer) > > > + > > > +Many work-in-progress patches for the above features exist. Contact > > > +Sascha Hauer <kernel@pengutronix.de> if you are interested in working > > > +on a specific feature. > > > + > > > +Please send any patches to Greg Kroah-Hartman <greg@kroah.com> and > > > > Greg Kroah-Hartman <gregkh@linuxfoundation.org>? > > Hm, right. I copied this from another TODO file. > > # find drivers/staging/ -name TODO | xargs grep greg@kroah.com | wc -l > 20 > > Maybe Gregs address should be dropped here, he will show up in > get-maintainers.pl anyway. > > Greg? Both end up in the same place, so it's not a big deal to leave my kroah.com address in there, but for new files, use linuxfoundation.org please. thanks, greg k-h
On 09/18/2012 11:52 PM, Sascha Hauer wrote: > On Tue, Sep 18, 2012 at 03:06:36PM -0700, Eric Nelson wrote: >> Hi Sascha, >> >> On 09/12/2012 03:31 AM, Sascha Hauer wrote: >>> From: Philipp Zabel<p.zabel@pengutronix.de> >>> >>> Signed-off-by: Philipp Zabel<p.zabel@pengutronix.de> >>> Signed-off-by: Sascha Hauer<s.hauer@pengutronix.de> >>> --- >>> .../bindings/staging/imx-drm/fsl-imx-drm.txt | 41 ++++++++++++++++++++ >>> 1 file changed, 41 insertions(+) >>> create mode 100644 Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt >>> >>> + >>> +example: >>> + >>> +display@di0 { >>> + compatible = "fsl,imx-parallel-display"; >>> + edid = [edid-data]; >>> + crtc =<&ipu 0>; >>> + interface-pix-fmt = "rgb24"; >>> +}; >> >> Do you have a working sample of [edid-data] for a parallel/LVDS/HDMI >> display or know a good way to produce one? > > No, we are using the of videomode helper, see > > http://www.mail-archive.com/devicetree-discuss@lists.ozlabs.org/msg18618.html > > It is not included here to not add a dependency on it. We are trying to > mainline this separately. > > Sascha > Thanks Sascha.