Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register
Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:
> In order to be able to share the control hardware block between
> VPUs use a syscon instead a ioremap it in the driver.
> To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
> phandle is not found look at 'ctrl' reg-name.
> With the method it becomes useless to provide a list of register
> names so remove it.
Sorry for putting a spoke in the wheel after many iterations of the
series.
We just discussed a way forward on how to handle the clocks and resets
provided by the blkctl block on i.MX8MM and later and it seems there is
a consensus on trying to provide virtual power domains from a blkctl
driver, controlling clocks and resets for the devices in the power
domain. I would like to avoid introducing yet another way of handling
the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
what we are planning to do on the later chip generations.
CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
virtual power domain thing a shot.
Regards,
Lucas
> Signed-off-by: Benjamin Gaignard
> Reviewed-by: Philipp Zabel
> ---
> version 9:
> - Corrections in commit message
>
> version 7:
> - Add Philipp reviewed-by tag.
> - Change syscon phandle name.
>
>
>
>
> version 5:
> - use syscon instead of VPU reset driver.
> - if DT doesn't provide syscon keep backward compatibilty by using
> 'ctrl' reg-name.
>
> drivers/staging/media/hantro/hantro.h | 5 +-
> drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 -
> 2 files changed, 34 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/staging/media/hantro/hantro.h
> b/drivers/staging/media/hantro/hantro.h
> index 6c1b888abe75..37b9ce04bd4e 100644
> --- a/drivers/staging/media/hantro/hantro.h
> +++ b/drivers/staging/media/hantro/hantro.h
> @@ -13,6 +13,7 @@
> #define HANTRO_H_
>
>
>
>
> #include
> +#include
> #include
> #include
> #include
> @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev)
> * @reg_bases: Mapped addresses of VPU registers.
> * @enc_base:Mapped address of VPU encoder register for
> convenience.
> * @dec_base:Mapped address of VPU decoder register for
> convenience.
> - * @ctrl_base: Mapped address of VPU control block.
> + * @ctrl_base: Regmap of VPU control block.
> * @vpu_mutex: Mutex to synchronize V4L2 calls.
> * @irqlock: Spinlock to synchronize access to data structures
> * shared with interrupt handlers.
> @@ -186,7 +187,7 @@ struct hantro_dev {
> void __iomem **reg_bases;
> void __iomem *enc_base;
> void __iomem *dec_base;
> - void __iomem *ctrl_base;
> + struct regmap *ctrl_base;
>
>
>
>
> struct mutex vpu_mutex; /* video_device lock */
> spinlock_t irqlock;
> diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> index c222de075ef4..8d0c3425234b 100644
> --- a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> +++ b/drivers/staging/media/hantro/imx8m_vpu_hw.c
> @@ -7,6 +7,7 @@
>
>
>
>
> #include
> #include
> +#include
>
>
>
>
> #include "hantro.h"
> #include "hantro_jpeg.h"
> @@ -24,30 +25,28 @@
> #define CTRL_G1_PP_FUSE 0x0c
> #define CTRL_G2_DEC_FUSE 0x10
>
>
>
>
> +static const struct regmap_config ctrl_regmap_ctrl = {
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 0x14,
> +};
> +
> static void imx8m_soft_reset(struct hantro_dev *vpu, u32 reset_bits)
> {
> - u32 val;
> -
> /* Assert */
> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
> - val &= ~reset_bits;
> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET, reset_bits, 0);
>
>
>
>
> udelay(2);
>
>
>
>
> /* Release */
> - val = readl(vpu->ctrl_base + CTRL_SOFT_RESET);
> - val |= reset_bits;
> - writel(val, vpu->ctrl_base + CTRL_SOFT_RESET);
> + regmap_update_bits(vpu->ctrl_base, CTRL_SOFT_RESET,
> +reset_bits, reset_bits);
> }
>
>
>
>
> static void imx8m_clk_enable(struct hantro_dev *vpu, u32 clock_bits)
> {
> - u32 val;
> -
> - val = readl(vpu->ctrl_base + CTRL_CLOCK_ENABLE);
> - val |= clock_bits;
> - writel(val, vpu->ctrl_base + CTRL_CLOCK_ENABLE);
> + regmap_update_bits(vpu->ctrl_base, CTRL_CLOCK_ENABLE,
> +clock_bits, clock_bits);
> }
>
>
>
>
> static int imx8mq_runtime_resume(struct hantro_dev *vpu)
> @@ -64,9 +63,9 @@ static int imx8mq_runtime_resume(struct hantro_dev *vpu)
> imx8m_clk_enable(vpu, CLOCK_G1 | CLOCK_G2);
>
>
>
>
> /* Set values of the fuse registers */
> - writel(0x, vpu->ctrl_base + CTRL_G1_DEC_FUSE);
> - writel(0x, vpu->ctrl_base + CTRL_G1_PP_FUSE);
> - write
Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register
Am Freitag, dem 16.04.2021 um 15:08 +0200 schrieb Benjamin Gaignard:
> Le 16/04/2021 à 12:54, Lucas Stach a écrit :
> > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard:
> > > In order to be able to share the control hardware block between
> > > VPUs use a syscon instead a ioremap it in the driver.
> > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl'
> > > phandle is not found look at 'ctrl' reg-name.
> > > With the method it becomes useless to provide a list of register
> > > names so remove it.
> > Sorry for putting a spoke in the wheel after many iterations of the
> > series.
> >
> > We just discussed a way forward on how to handle the clocks and resets
> > provided by the blkctl block on i.MX8MM and later and it seems there is
> > a consensus on trying to provide virtual power domains from a blkctl
> > driver, controlling clocks and resets for the devices in the power
> > domain. I would like to avoid introducing yet another way of handling
> > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with
> > what we are planning to do on the later chip generations.
> >
> > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this
> > virtual power domain thing a shot.
>
> That could replace the 3 first patches and Dt patche of this series
> but that will not impact the hevc part, so I wonder if pure hevc patches
> could be merged anyway ?
> They are reviewed and don't depend of how the ctrl block is managed.
I'm not really in a position to give any informed opinion about that
hvec patches, as I only skimmed them, but I don't see any reason to
delay patches 04-11 from this series until the i.MX8M platform issues
are sorted. AFAICS those things are totally orthogonal.
Regards,
Lucas
> >
> > Regards,
> > Lucas
> >
> > > Signed-off-by: Benjamin Gaignard
> > > Reviewed-by: Philipp Zabel
> > > ---
> > > version 9:
> > > - Corrections in commit message
> > >
> > > version 7:
> > > - Add Philipp reviewed-by tag.
> > > - Change syscon phandle name.
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > version 5:
> > > - use syscon instead of VPU reset driver.
> > > - if DT doesn't provide syscon keep backward compatibilty by using
> > > 'ctrl' reg-name.
> > >
> > > drivers/staging/media/hantro/hantro.h | 5 +-
> > > drivers/staging/media/hantro/imx8m_vpu_hw.c | 52 -
> > > 2 files changed, 34 insertions(+), 23 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/hantro/hantro.h
> > > b/drivers/staging/media/hantro/hantro.h
> > > index 6c1b888abe75..37b9ce04bd4e 100644
> > > --- a/drivers/staging/media/hantro/hantro.h
> > > +++ b/drivers/staging/media/hantro/hantro.h
> > > @@ -13,6 +13,7 @@
> > > #define HANTRO_H_
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > #include
> > > +#include
> > > #include
> > > #include
> > > #include
> > > @@ -167,7 +168,7 @@ hantro_vdev_to_func(struct video_device *vdev)
> > > * @reg_bases: Mapped addresses of VPU registers.
> > > * @enc_base: Mapped address of VPU encoder register for
> > > convenience.
> > > * @dec_base: Mapped address of VPU decoder register for
> > > convenience.
> > > - * @ctrl_base: Mapped address of VPU control block.
> > > + * @ctrl_base: Regmap of VPU control block.
> > > * @vpu_mutex: Mutex to synchronize V4L2 calls.
> > > * @irqlock:Spinlock to synchronize access to data
> > > structures
> > > * shared with interrupt handlers.
> > > @@ -186,7 +187,7 @@ struct hantro_dev {
> > > void __iomem **reg_bases;
> > > void __iomem *enc_base;
> > > void __iomem *dec_base;
> > > - void __iomem *ctrl_base;
> > > + struct regmap *ctrl_base;
> > >
> > >
> > >
> > >
> > >
> > >
> > >
> > > struct mutex vpu_mutex; /* video_device lock */
> > > spinlock_t irqlock;
> > > diff --git a/drivers/staging/media/hantro/imx8m_vpu_hw.c
> > > b/drivers/staging/media/hantro/imx8m_
Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register
Hi Ezequiel, Am Sonntag, dem 16.05.2021 um 19:40 -0300 schrieb Ezequiel Garcia: > Hi Lucas, > > On Fri, 2021-04-16 at 12:54 +0200, Lucas Stach wrote: > > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > > In order to be able to share the control hardware block between > > > VPUs use a syscon instead a ioremap it in the driver. > > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > > phandle is not found look at 'ctrl' reg-name. > > > With the method it becomes useless to provide a list of register > > > names so remove it. > > > > Sorry for putting a spoke in the wheel after many iterations of the > > series. > > > > We just discussed a way forward on how to handle the clocks and resets > > provided by the blkctl block on i.MX8MM and later and it seems there is > > a consensus on trying to provide virtual power domains from a blkctl > > driver, controlling clocks and resets for the devices in the power > > domain. I would like to avoid introducing yet another way of handling > > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > > what we are planning to do on the later chip generations. > > > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > > virtual power domain thing a shot. > > > > It seems the i.MX8MM BLK-CTL series are moving forward: > > https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=479175 > > ... but I'm unable to wrap my head around how this affects the > devicetree VPU modelling for i.MX8MQ (and also i.MX8MM, i.MX8MP, ...). > > For the i.MX8MQ we want to have the same virtual power-domains provided by a BLK-CTRL driver for the VPUs, as on i.MX8MM. This way we should be able to use the same DT bindings for the VPUs on i.MX8MQ and i.MX8MM, even though the SoC integration with the blk-ctrl is a little different. > Can you clarify that? > I'm planning on sending some patches adding i.MX8MQ VPU support to the BLK-CTRL driver in the next few days. I guess that should clarify things. :) Regards, Lucas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v9 03/13] media: hantro: Use syscon instead of 'ctrl' register
Am Montag, dem 17.05.2021 um 10:23 -0300 schrieb Ezequiel Garcia: > On Mon, 2021-05-17 at 12:52 +0200, Lucas Stach wrote: > > Hi Ezequiel, > > > > Am Sonntag, dem 16.05.2021 um 19:40 -0300 schrieb Ezequiel Garcia: > > > Hi Lucas, > > > > > > On Fri, 2021-04-16 at 12:54 +0200, Lucas Stach wrote: > > > > Am Mittwoch, dem 07.04.2021 um 09:35 +0200 schrieb Benjamin Gaignard: > > > > > In order to be able to share the control hardware block between > > > > > VPUs use a syscon instead a ioremap it in the driver. > > > > > To keep the compatibility with older DT if 'nxp,imx8mq-vpu-ctrl' > > > > > phandle is not found look at 'ctrl' reg-name. > > > > > With the method it becomes useless to provide a list of register > > > > > names so remove it. > > > > > > > > Sorry for putting a spoke in the wheel after many iterations of the > > > > series. > > > > > > > > We just discussed a way forward on how to handle the clocks and resets > > > > provided by the blkctl block on i.MX8MM and later and it seems there is > > > > a consensus on trying to provide virtual power domains from a blkctl > > > > driver, controlling clocks and resets for the devices in the power > > > > domain. I would like to avoid introducing yet another way of handling > > > > the blkctl and thus would like to align the i.MX8MQ VPU blkctl with > > > > what we are planning to do on the later chip generations. > > > > > > > > CC'ing Jacky Bai and Peng Fan from NXP, as they were going to give this > > > > virtual power domain thing a shot. > > > > > > > > > > It seems the i.MX8MM BLK-CTL series are moving forward: > > > > > > https://patchwork.kernel.org/project/linux-arm-kernel/list/?series=479175 > > > > > > ... but I'm unable to wrap my head around how this affects the > > > devicetree VPU modelling for i.MX8MQ (and also i.MX8MM, i.MX8MP, ...). > > > > > > > > For the i.MX8MQ we want to have the same virtual power-domains provided > > by a BLK-CTRL driver for the VPUs, as on i.MX8MM. This way we should be > > able to use the same DT bindings for the VPUs on i.MX8MQ and i.MX8MM, > > even though the SoC integration with the blk-ctrl is a little > > different. > > > > AFAICS, there's not support for i.MX8MP VPU power domains. I suppose > we should make sure we'll be able to cover those as well. > > Will i.MX8MP need its own driver as well? > > I haven't looked too closely at the 8MP VPU subsystem yet, but I expect it to be slightly different again so it will need changes to the blk- ctrl driver. But that's the whole point of this virtual power domain exercise: abstract away the SoC specific things in the blk-ctrl driver, so the VPU driver doesn't need to care about them. Regards, Lucas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH V4 5/6] dw-hdmi: add support for multi byte register width access
Am Freitag, den 07.11.2014, 19:35 +0800 schrieb Andy Yan:
> On rockchip rk3288, only word(32-bit) accesses are
> permitted for hdmi registers. Byte width access (writeb,
> readb) generates an imprecise external abort.
>
> Signed-off-by: Andy Yan
> ---
> drivers/gpu/drm/bridge/dw_hdmi.c | 49
> +---
> 1 file changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c
> b/drivers/gpu/drm/bridge/dw_hdmi.c
> index df76a8c..9867642 100644
> --- a/drivers/gpu/drm/bridge/dw_hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw_hdmi.c
> @@ -126,19 +126,42 @@ struct dw_hdmi {
[...]
> + u32 val;
> +
> + if (!of_property_read_u32(np, "reg-io-width", &val)) {
> + switch (val) {
> + case 4:
> + hdmi->write = dw_hdmi_writel;
> + hdmi->read = dw_hdmi_readl;
> + hdmi->reg_shift = 2;
> + break;
> + default:
> + hdmi->write = dw_hdmi_writeb;
> + hdmi->read = dw_hdmi_readb;
> + hdmi->reg_shift = 0;
> + break;
> + }
> + } else {
> + hdmi->write = dw_hdmi_writeb;
> + hdmi->read = dw_hdmi_readb;
> + hdmi->reg_shift = 0;
> + }
>
> ddc_node = of_parse_phandle(np, "ddc-i2c-bus", 0);
> if (ddc_node) {
This should throw an error if the property value in devicetree is not
recognized. This could be simplified like this:
u32 val = 1;
// this won't touch val if it can't find the property
of_property_read_u32(np, "reg-io-width", &val)
switch (val) {
case 4:
hdmi->write = dw_hdmi_writel;
hdmi->read = dw_hdmi_readl;
hdmi->reg_shift = 2;
break;
case 1:
hdmi->write = dw_hdmi_writeb;
hdmi->read = dw_hdmi_readb;
hdmi->reg_shift = 0;
break;
default:
dev_err(dev, "unrecognized value for reg-io-width");
// error handling
}
Also the DT binding doc for this property is missing.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v7 5/7] drm: bridge/dw-hdmi: add support for multi byte register width access
Am Dienstag, den 11.11.2014, 14:20 + schrieb Zubair Lutfullah
Kakakhel:
> Hi Andy,
>
> This patch adds the reg-io-width binding.
>
> Hence the binding patch should come before it.
>
>
> On 11/11/14 12:53, Andy Yan wrote:
> > On rockchip rk3288, only word(32-bit) accesses are
> > permitted for hdmi registers. Byte width accesses (writeb,
> > readb) generate an imprecise external abort.
> >
> > Signed-off-by: Andy Yan
> >
> > ---
> >
> > }
> >
> > static void hdmi_modb(struct dw_hdmi *hdmi, u8 data, u8 mask, unsigned reg)
> > @@ -1499,6 +1527,23 @@ static int dw_hdmi_bind(struct device *dev, struct
> > device *master, void *data)
> > struct device_node *ddc_node;
> > struct resource *iores;
> > int ret, irq;
> > + u32 val = 1;
> > +
> > + of_property_read_u32(np, "reg-io-width", &val);
> > +
> > + switch (val) {
> > + case 4:
> > + hdmi->write = dw_hdmi_writel;
> > + hdmi->read = dw_hdmi_readl;
> > + break;
> > + case 1:
> > + hdmi->write = dw_hdmi_writeb;
> > + hdmi->read = dw_hdmi_readb;
> > + break;
> > + default:
> > + dev_err(dev, "reg-io-width must be 1 or 4\n");
> > + return -EINVAL;
> > + }
>
> The binding patch says this is an optional property.
> But here if undefined it returns -EINVAL.
>
> I would keep it optional and default it to byte access.
That's exactly what the patch does. val is initialized to 1, which is
not changed if the property could not be found in the DT. The default
case will only be taken if the property is present in DT but has any
other value than 1 or 4, which is an error.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v5 00/11] imx-drm dt bindings
Am Dienstag, den 11.03.2014, 21:27 +0800 schrieb Shawn Guo:
> On Tue, Mar 11, 2014 at 12:42:08PM +0100, Philipp Zabel wrote:
> > Hi Shawn,
> >
> > Am Dienstag, den 11.03.2014, 11:46 +0800 schrieb Shawn Guo:
> > > On Wed, Mar 05, 2014 at 10:20:51AM +0100, Philipp Zabel wrote:
> > > > Hi,
> > > >
> > > > this latest version of the imx-drm DT binding patches applies
> > > > on top of staging-next and also depends on the OF graph binding
> > > > patchset that moves the v4l2_of helpers to drivers/of.
> > > > Currently, the two patchsets are also available at:
> > > > git://git.pengutronix.de/git/pza/linux.git topic/of-graph
> > > > git://git.pengutronix.de/git/pza/linux.git topic/imx-drm-dt
> > >
> > > Hi Philipp,
> > >
> > > I just came across a couple problems when testing the series on
> > > my imx6dl-sabresd board in dual display case - HDMI + LVDS. I tested it
> > > using Russell's branch below, which I believe has all the pieces put
> > > together.
> > >
> > > git://ftp.arm.linux.org.uk/~rmk/linux-arm.git imx-drm-staging
> > >
> > > - When I enable HDMI and LVDS support in both kernel build and device
> > > tree, HDMI seems working fine but LVDS color is corrupted quite badly.
> > >
> > > - When I enable HDMI and LVDS support in kernel build but only LVDS in
> > > device tree (keep HDMI disabled in device tree by not changing
> > > 'status' of HDMI node to 'okay'), LVDS does not even work. In this
> > > case, it seems that the binding of display-subsystem does not succeed.
> >
> > Can you check if you get the bound messages from
> > drivers/base/component.c:
> >
> > imx-drm display-subsystem.11: bound imx-ipuv3-crtc.0 (ops ipu_crtc_ops)
> > imx-drm display-subsystem.11: bound imx-ipuv3-crtc.1 (ops ipu_crtc_ops)
> > imx-drm display-subsystem.11: bound ldb.10 (ops imx_ldb_ops)
> >
> > I have tried this branch with a Phytec phyFLEX i.MX6S on PBAB01
> > baseboard with EDT 800x480 LVDS panel, and it seems to work.
> > The check in drivers/staging/imx-drm/imx-drm-core.c:675 should make sure
> > that unavailable (status="disabled") devices are just skipped.
>
> Sorry, Philipp. The setup in my report is incorrect. The correct setup
> to see this issue consists of:
>
> 1) build a kernel without HDMI support, i.e. !CONFIG_DRM_IMX_HDMI
>
> 2) enable HDMI device in DT, i.e. adding the lines below in board dts.
>
> &hdmi {
> status = "okay";
> };
>
> Sorry for the confusion.
>
This isn't a supported use-case, the componentized device stuff is there
exactly to ensure that the DRM device is only brought up after all
active components have an driver attached to them.
Regards,
Lucas
--
Pengutronix e.K. | Lucas Stach |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 4/4] imx-drm: imx-hdmi: don't erroneously detect HDMI displays
While having a CEA mode is one prerequisite for using
HDMI transmit mode, we also have to check if the display
is actually capable of HDMI input. Fall back to DVI mode
otherwise.
Signed-off-by: Lucas Stach
---
drivers/staging/imx-drm/imx-hdmi.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-hdmi.c
b/drivers/staging/imx-drm/imx-hdmi.c
index 7d407e917786..28d1cdd12cb4 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -1218,13 +1218,10 @@ static int imx_hdmi_setup(struct imx_hdmi *hdmi, struct
drm_display_mode *mode)
hdmi->vic = drm_match_cea_mode(mode);
- if (!hdmi->vic) {
- dev_dbg(hdmi->dev, "Non-CEA mode used in HDMI\n");
- hdmi->hdmi_data.video_mode.mdvi = true;
- } else {
- dev_dbg(hdmi->dev, "CEA mode used vic=%d\n", hdmi->vic);
+ if (drm_detect_hdmi_monitor(hdmi->edid) && hdmi->vic)
hdmi->hdmi_data.video_mode.mdvi = false;
- }
+ else
+ hdmi->hdmi_data.video_mode.mdvi = true;
if ((hdmi->vic == 6) || (hdmi->vic == 7) ||
(hdmi->vic == 21) || (hdmi->vic == 22) ||
@@ -1273,7 +1270,7 @@ static int imx_hdmi_setup(struct imx_hdmi *hdmi, struct
drm_display_mode *mode)
if (hdmi->hdmi_data.video_mode.mdvi)
dev_dbg(hdmi->dev, "%s DVI mode\n", __func__);
else {
- dev_dbg(hdmi->dev, "%s CEA mode\n", __func__);
+ dev_dbg(hdmi->dev, "%s CEA mode VIC=%d\n", __func__, hdmi->vic);
/* HDMI Initialization Step E - Configure audio */
hdmi_clk_regenerator_update_pixel_clock(hdmi);
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 2/4] staging: imx-hdmi: correct connector detect and hotplug
Make sure that we probe for a display on detect regardless
of previous hotplug events. Don't handle connector
hotplug state ourselves, but let DRM do the right thing
for us. This brings our hotplug handling in line with
what other DRM drivers do.
Signed-off-by: Lucas Stach
---
drivers/staging/imx-drm/imx-hdmi.c | 73 +-
1 file changed, 33 insertions(+), 40 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-hdmi.c
b/drivers/staging/imx-drm/imx-hdmi.c
index 80b91b1e225c..22cfdfc5ef74 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -32,8 +32,6 @@
#include "imx-hdmi.h"
#include "imx-drm.h"
-#define HDMI_EDID_LEN 512
-
#define RGB0
#define YCBCR444 1
#define YCBCR422_16BITS2
@@ -120,12 +118,10 @@ struct imx_hdmi {
struct clk *isfr_clk;
struct clk *iahb_clk;
- enum drm_connector_status connector_status;
-
struct hdmi_data_info hdmi_data;
int vic;
- u8 edid[HDMI_EDID_LEN];
+ struct edid *edid;
bool cable_plugin;
bool phy_enabled;
@@ -1386,32 +1382,46 @@ static enum drm_connector_status
imx_hdmi_connector_detect(struct drm_connector
struct imx_hdmi *hdmi = container_of(connector, struct imx_hdmi,
connector);
- return hdmi->connector_status;
+ /* free previous EDID block */
+ if (hdmi->edid) {
+ drm_mode_connector_update_edid_property(connector, NULL);
+ kfree(hdmi->edid);
+ }
+
+ if (hdmi->ddc && drm_probe_ddc(hdmi->ddc)) {
+ hdmi->edid = drm_get_edid(connector, hdmi->ddc);
+ if (hdmi->edid) {
+ drm_mode_connector_update_edid_property(connector,
+ hdmi->edid);
+ dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
+ hdmi->edid->width_cm, hdmi->edid->height_cm);
+
+ return connector_status_connected;
+ }
+ }
+
+ /* if EDID probing fails, try if we can sense an attached display */
+ if (hdmi_readb(hdmi, HDMI_PHY_STAT0) & 0xf0)
+ return connector_status_connected;
+
+ return connector_status_disconnected;
}
static int imx_hdmi_connector_get_modes(struct drm_connector *connector)
{
struct imx_hdmi *hdmi = container_of(connector, struct imx_hdmi,
connector);
- struct edid *edid;
- int ret;
+ int ret = 0;
- if (!hdmi->ddc)
- return 0;
-
- edid = drm_get_edid(connector, hdmi->ddc);
- if (edid) {
- dev_dbg(hdmi->dev, "got edid: width[%d] x height[%d]\n",
- edid->width_cm, edid->height_cm);
-
- drm_mode_connector_update_edid_property(connector, edid);
- ret = drm_add_edid_modes(connector, edid);
- kfree(edid);
+ if (hdmi->edid) {
+ ret = drm_add_edid_modes(connector, hdmi->edid);
+ /* Store the ELD */
+ drm_edid_to_eld(connector, hdmi->edid);
} else {
- dev_dbg(hdmi->dev, "failed to get edid\n");
+ dev_dbg(hdmi->dev, "failed to get edid modes\n");
}
- return 0;
+ return ret;
}
static struct drm_encoder *imx_hdmi_connector_best_encoder(struct drm_connector
@@ -1519,27 +1529,10 @@ static irqreturn_t imx_hdmi_irq(int irq, void *dev_id)
u8 phy_int_pol;
intr_stat = hdmi_readb(hdmi, HDMI_IH_PHY_STAT0);
-
phy_int_pol = hdmi_readb(hdmi, HDMI_PHY_POL0);
if (intr_stat & hdmi->sink_detect_status) {
- int pol_bit = hdmi->sink_detect_polarity;
-
- if (phy_int_pol & pol_bit) {
- dev_dbg(hdmi->dev, "EVENT=plugin\n");
-
- hdmi_modb(hdmi, 0, pol_bit, HDMI_PHY_POL0);
-
- hdmi->connector_status = connector_status_connected;
- imx_hdmi_poweron(hdmi);
- } else {
- dev_dbg(hdmi->dev, "EVENT=plugout\n");
-
- hdmi_modb(hdmi, pol_bit, pol_bit, HDMI_PHY_POL0);
-
- hdmi->connector_status = connector_status_disconnected;
- imx_hdmi_poweroff(hdmi);
- }
+ hdmi_modb(hdmi, ~phy_int_pol, ~hdmi->sink_detect_mask,
HDMI_PHY_POL0);
drm_helper_hpd_irq_event(hdmi->connector.dev);
}
@@ -1611,7 +1604,6 @@ static int imx_hdmi_bind(struct device *dev, struct
device *master, void *data)
[PATCH 1/4] staging: imx-hdmi: use rx sense pins for plug detection if hpd is unreliable
From: Philipp Zabel
Due to the voltage divider on the HPD line, the HDMI connector on
imx6q-sabrelite doesn't reliably detect connected DVI monitors.
This patch allows to use the RX_SENSE0 signal as a workaround when
enabled by a boolean device tree property 'hpd-unreliable'.
Signed-off-by: Philipp Zabel
Signed-off-by: Lucas Stach
---
drivers/staging/imx-drm/imx-hdmi.c | 37 ++---
1 file changed, 26 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/imx-drm/imx-hdmi.c
b/drivers/staging/imx-drm/imx-hdmi.c
index d47dedd2cdb4..80b91b1e225c 100644
--- a/drivers/staging/imx-drm/imx-hdmi.c
+++ b/drivers/staging/imx-drm/imx-hdmi.c
@@ -134,6 +134,9 @@ struct imx_hdmi {
struct regmap *regmap;
struct i2c_adapter *ddc;
void __iomem *regs;
+ u8 sink_detect_polarity;
+ u8 sink_detect_status;
+ u8 sink_detect_mask;
unsigned int sample_rate;
int ratio;
@@ -1307,10 +1310,10 @@ static int imx_hdmi_fb_registered(struct imx_hdmi *hdmi)
HDMI_PHY_I2CM_CTLINT_ADDR);
/* enable cable hot plug irq */
- hdmi_writeb(hdmi, (u8)~HDMI_PHY_HPD, HDMI_PHY_MASK0);
+ hdmi_writeb(hdmi, hdmi->sink_detect_mask, HDMI_PHY_MASK0);
/* Clear Hotplug interrupts */
- hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD, HDMI_IH_PHY_STAT0);
+ hdmi_writeb(hdmi, hdmi->sink_detect_status, HDMI_IH_PHY_STAT0);
return 0;
}
@@ -1382,6 +1385,7 @@ static enum drm_connector_status
imx_hdmi_connector_detect(struct drm_connector
{
struct imx_hdmi *hdmi = container_of(connector, struct imx_hdmi,
connector);
+
return hdmi->connector_status;
}
@@ -1518,19 +1522,20 @@ static irqreturn_t imx_hdmi_irq(int irq, void *dev_id)
phy_int_pol = hdmi_readb(hdmi, HDMI_PHY_POL0);
- if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
- if (phy_int_pol & HDMI_PHY_HPD) {
+ if (intr_stat & hdmi->sink_detect_status) {
+ int pol_bit = hdmi->sink_detect_polarity;
+
+ if (phy_int_pol & pol_bit) {
dev_dbg(hdmi->dev, "EVENT=plugin\n");
- hdmi_modb(hdmi, 0, HDMI_PHY_HPD, HDMI_PHY_POL0);
+ hdmi_modb(hdmi, 0, pol_bit, HDMI_PHY_POL0);
hdmi->connector_status = connector_status_connected;
imx_hdmi_poweron(hdmi);
} else {
dev_dbg(hdmi->dev, "EVENT=plugout\n");
- hdmi_modb(hdmi, HDMI_PHY_HPD, HDMI_PHY_HPD,
- HDMI_PHY_POL0);
+ hdmi_modb(hdmi, pol_bit, pol_bit, HDMI_PHY_POL0);
hdmi->connector_status = connector_status_disconnected;
imx_hdmi_poweroff(hdmi);
@@ -1539,7 +1544,7 @@ static irqreturn_t imx_hdmi_irq(int irq, void *dev_id)
}
hdmi_writeb(hdmi, intr_stat, HDMI_IH_PHY_STAT0);
- hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0);
+ hdmi_writeb(hdmi, ~hdmi->sink_detect_status, HDMI_IH_MUTE_PHY_STAT0);
return IRQ_HANDLED;
}
@@ -1691,14 +1696,24 @@ static int imx_hdmi_bind(struct device *dev, struct
device *master, void *data)
*/
hdmi_init_clk_regenerator(hdmi);
+ hdmi->sink_detect_status = HDMI_IH_PHY_STAT0_HPD;
+ hdmi->sink_detect_polarity = HDMI_PHY_HPD;
+ hdmi->sink_detect_mask = ~HDMI_PHY_HPD;
+
+ if (of_property_read_bool(np, "hpd-unreliable")) {
+ hdmi->sink_detect_status = HDMI_IH_PHY_STAT0_RX_SENSE0;
+ hdmi->sink_detect_polarity = HDMI_PHY_RX_SENSE0;
+ hdmi->sink_detect_mask = ~HDMI_PHY_RX_SENSE0;
+ }
+
/*
* Configure registers related to HDMI interrupt
* generation before registering IRQ.
*/
- hdmi_writeb(hdmi, HDMI_PHY_HPD, HDMI_PHY_POL0);
+ hdmi_writeb(hdmi, hdmi->sink_detect_polarity, HDMI_PHY_POL0);
/* Clear Hotplug interrupts */
- hdmi_writeb(hdmi, HDMI_IH_PHY_STAT0_HPD, HDMI_IH_PHY_STAT0);
+ hdmi_writeb(hdmi, hdmi->sink_detect_status, HDMI_IH_PHY_STAT0);
ret = imx_hdmi_fb_registered(hdmi);
if (ret)
@@ -1709,7 +1724,7 @@ static int imx_hdmi_bind(struct device *dev, struct
device *master, void *data)
goto err_iahb;
/* Unmute interrupts */
- hdmi_writeb(hdmi, ~HDMI_IH_PHY_STAT0_HPD, HDMI_IH_MUTE_PHY_STAT0);
+ hdmi_writeb(hdmi, ~hdmi->sink_detect_status, HDMI_IH_MUTE_PHY_STAT0);
dev_set_drvdata(dev, hdmi);
--
1.9.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH 3/4] staging: imx-hdmi: clear all hotplug IRQs on bind
Makes sure we don't receive a stray IRQ on startup. Signed-off-by: Lucas Stach --- drivers/staging/imx-drm/imx-hdmi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/imx-drm/imx-hdmi.c b/drivers/staging/imx-drm/imx-hdmi.c index 22cfdfc5ef74..7d407e917786 100644 --- a/drivers/staging/imx-drm/imx-hdmi.c +++ b/drivers/staging/imx-drm/imx-hdmi.c @@ -1705,7 +1705,7 @@ static int imx_hdmi_bind(struct device *dev, struct device *master, void *data) hdmi_writeb(hdmi, hdmi->sink_detect_polarity, HDMI_PHY_POL0); /* Clear Hotplug interrupts */ - hdmi_writeb(hdmi, hdmi->sink_detect_status, HDMI_IH_PHY_STAT0); + hdmi_writeb(hdmi, 0x3d, HDMI_IH_PHY_STAT0); ret = imx_hdmi_fb_registered(hdmi); if (ret) -- 1.9.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: imx-hdmi: correct connector detect and hotplug
Am Sonntag, den 13.04.2014, 15:58 +0100 schrieb Russell King - ARM Linux: > On Fri, Apr 11, 2014 at 04:13:33PM +0200, Lucas Stach wrote: > > Make sure that we probe for a display on detect regardless > > of previous hotplug events. Don't handle connector > > hotplug state ourselves, but let DRM do the right thing > > for us. This brings our hotplug handling in line with > > what other DRM drivers do. > > Why should working setups have to pay the price for faulty setups when we > can adequately detect the hotplug signal on iMX SoCs when it's correctly > wired? > > By "price" I mean - if we end up having to poll the connector, we end up > calling the i2c functions, and the i2c functions on iMX use a fixed > timeout of 100ms. That means the context which runs the > imx_hdmi_connector_detect() function is forced to sleep for 100ms. If > that's being run as part of a softirq (eg, via a work struct), that's > bad news because that could be any thread in the system. > > The "price" should only be paid by those implementations where the hotplug > signal is not correctly wired. > This change is not related to broken systems. It just uses the DRM framework as intended. The detect() callback, which triggers the EDID fetch will only be called by DRM when a hotplug event was received, or if someone (e.g. kms_fb_helper, or userspace) explicitly requests to poll the connector. Not doing so is working around the DRM framework, not using it. So as mentioned this change just brings us in line with what other DRM drivers do to handle hotplug and connector detect. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: imx-hdmi: correct connector detect and hotplug
Am Montag, den 14.04.2014, 10:10 +0100 schrieb Russell King - ARM Linux: > On Mon, Apr 14, 2014 at 10:42:32AM +0200, Lucas Stach wrote: > > Am Sonntag, den 13.04.2014, 15:58 +0100 schrieb Russell King - ARM > > Linux: > > > On Fri, Apr 11, 2014 at 04:13:33PM +0200, Lucas Stach wrote: > > > > Make sure that we probe for a display on detect regardless > > > > of previous hotplug events. Don't handle connector > > > > hotplug state ourselves, but let DRM do the right thing > > > > for us. This brings our hotplug handling in line with > > > > what other DRM drivers do. > > > > > > Why should working setups have to pay the price for faulty setups when we > > > can adequately detect the hotplug signal on iMX SoCs when it's correctly > > > wired? > > > > > > By "price" I mean - if we end up having to poll the connector, we end up > > > calling the i2c functions, and the i2c functions on iMX use a fixed > > > timeout of 100ms. That means the context which runs the > > > imx_hdmi_connector_detect() function is forced to sleep for 100ms. If > > > that's being run as part of a softirq (eg, via a work struct), that's > > > bad news because that could be any thread in the system. > > > > > > The "price" should only be paid by those implementations where the hotplug > > > signal is not correctly wired. > > > > > This change is not related to broken systems. It just uses the DRM > > framework as intended. The detect() callback, which triggers the EDID > > fetch will only be called by DRM when a hotplug event was received, or > > if someone (e.g. kms_fb_helper, or userspace) explicitly requests to > > poll the connector. > > > > Not doing so is working around the DRM framework, not using it. So as > > mentioned this change just brings us in line with what other DRM drivers > > do to handle hotplug and connector detect. > > I totally disagree with that. What we're doing today using HPD to > detect connection is entirely in keeping with DRM and the HDMI spec, > and is more correct than your solution using EDID to detect the > presence of a connection. > > HPD in HDMI indicates that the EDID is available for reading. There > is no need what so ever to try reading the EDID to detect whether > a device is present. > > Moreover, the HDMI spec does not say what state the DDC signals will > be when the sink is powered off - it seems to me that it is entirely > reasonable when HPD is lowered due to the sink being powered off that > the DDC signals may be clamped to logic zero by ESD diodes in the sink, > which would cause problems when trying to detect by reading the EDID. > > Moreover, it is quite legal for a sink to modify the contents of its > EEPROM - and it can do this by manipulating the DDC signals itself. > Polling the EDID would open the possibilities of races, reading the > EDID before the sink had finished updating it. > And that's exactly what happens now. We do not poll the EDID in any way, until we are explicitly asked to do so, which happens only very few occasions. Please go back and read the code after this patch. What we do now in the regular case (nobody is calling detect() explicitly) is the following: 1. We wait for the HDMI irq to signal 2. If we got a HDMI hpd event we call drm_helper_hpd_irq_event() 3. In response to this event DRM calls our detect() function, which tries to fetch the EDID. 4. If an EDID is found we report a connected display. This sequence is completely in line with what the HDMI spec says and what you demand has to be done. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 2/4] staging: imx-hdmi: correct connector detect and hotplug
Am Montag, den 14.04.2014, 11:09 +0100 schrieb Russell King - ARM Linux: > On Mon, Apr 14, 2014 at 11:38:43AM +0200, Lucas Stach wrote: > > Am Montag, den 14.04.2014, 10:10 +0100 schrieb Russell King - ARM Linux: > > > On Mon, Apr 14, 2014 at 10:42:32AM +0200, Lucas Stach wrote: > > > > Am Sonntag, den 13.04.2014, 15:58 +0100 schrieb Russell King - ARM > > > > Linux: > > > > > On Fri, Apr 11, 2014 at 04:13:33PM +0200, Lucas Stach wrote: > > > > > > Make sure that we probe for a display on detect regardless > > > > > > of previous hotplug events. Don't handle connector > > > > > > hotplug state ourselves, but let DRM do the right thing > > > > > > for us. This brings our hotplug handling in line with > > > > > > what other DRM drivers do. > > > > > > > > > > Why should working setups have to pay the price for faulty setups > > > > > when we > > > > > can adequately detect the hotplug signal on iMX SoCs when it's > > > > > correctly > > > > > wired? > > > > > > > > > > By "price" I mean - if we end up having to poll the connector, we end > > > > > up > > > > > calling the i2c functions, and the i2c functions on iMX use a fixed > > > > > timeout of 100ms. That means the context which runs the > > > > > imx_hdmi_connector_detect() function is forced to sleep for 100ms. If > > > > > that's being run as part of a softirq (eg, via a work struct), that's > > > > > bad news because that could be any thread in the system. > > > > > > > > > > The "price" should only be paid by those implementations where the > > > > > hotplug > > > > > signal is not correctly wired. > > > > > > > > > This change is not related to broken systems. It just uses the DRM > > > > framework as intended. The detect() callback, which triggers the EDID > > > > fetch will only be called by DRM when a hotplug event was received, or > > > > if someone (e.g. kms_fb_helper, or userspace) explicitly requests to > > > > poll the connector. > > > > > > > > Not doing so is working around the DRM framework, not using it. So as > > > > mentioned this change just brings us in line with what other DRM drivers > > > > do to handle hotplug and connector detect. > > > > > > I totally disagree with that. What we're doing today using HPD to > > > detect connection is entirely in keeping with DRM and the HDMI spec, > > > and is more correct than your solution using EDID to detect the > > > presence of a connection. > > > > > > HPD in HDMI indicates that the EDID is available for reading. There > > > is no need what so ever to try reading the EDID to detect whether > > > a device is present. > > > > > > Moreover, the HDMI spec does not say what state the DDC signals will > > > be when the sink is powered off - it seems to me that it is entirely > > > reasonable when HPD is lowered due to the sink being powered off that > > > the DDC signals may be clamped to logic zero by ESD diodes in the sink, > > > which would cause problems when trying to detect by reading the EDID. > > > > > > Moreover, it is quite legal for a sink to modify the contents of its > > > EEPROM - and it can do this by manipulating the DDC signals itself. > > > Polling the EDID would open the possibilities of races, reading the > > > EDID before the sink had finished updating it. > > > > > > > And that's exactly what happens now. We do not poll the EDID in any way, > > until we are explicitly asked to do so, which happens only very few > > occasions. > > > > Please go back and read the code after this patch. What we do now in the > > regular case (nobody is calling detect() explicitly) is the following: > > Now *you* please go back and read what you said about kms/userspace being > able to poll the connector, thereby causing an EDID read attempt while > HPD may not be active. > Yes, userspace may trigger an explicit detect because it might suspect that a sink is present while it has not received any HP event. What userspace expects to happen in this situation is an explicit poll of the connector regardless of the HP status. If you then just report the connector as disconnected because you didn't receive a HP event before, you break the use-case for which userspace is calling an explicit detect in the first place. The userspace expects to encounter potentially long timeouts when calling this function. For properly working setups userspace never does this, as the HP event is enough to handle those. Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v6 17/39] platform: add video-multiplexer subdevice driver
Am Mittwoch, den 05.04.2017, 13:18 +0200 schrieb Pavel Machek: > Hi! > > > + * video stream multiplexer controlled via gpio or syscon > > + * > > + * Copyright (C) 2013 Pengutronix, Sascha Hauer > > + * Copyright (C) 2016 Pengutronix, Philipp Zabel > > This is actually quite interesting. Same email address for two > people... > > Plus, I believe this wants to say that copyright is with Pengutronix, > not Sascha and Philipp. In that case you probably want to list > copyright and authors separately? > Nope, copyright doesn't get transferred to the employer within the rules of the German "Urheberrecht", but stays at the original author of the code. Same email is just to ensure that any requests regarding this code get routed to the right people, even if someone leaves the company or is temporarily unavailable. kernel@ is a list for the Pengutronix kernel team. Regards, Lucas ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v12 1/2] kernel.h: add u64_to_user_ptr()
Am Donnerstag, den 21.04.2016, 12:38 -0300 schrieb Gustavo Padovan: > From: Gustavo Padovan > > This function had copies in 3 different files. Unify them in kernel.h. > > Cc: Joe Perches > Cc: Andrew Morton > Cc: David Airlie > Cc: Daniel Vetter > Cc: Rob Clark > Signed-off-by: Gustavo Padovan > Though I normally prefer static inline functions, I see the benefits of using the macro form here. For the etnaviv part: Acked-by: Lucas Stach ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] drm/etnaviv: add thermal dependency
Hi Arnd,
Am Mittwoch, den 26.07.2017, 15:53 +0200 schrieb Arnd Bergmann:
> When CONFIG_THERMAL is enabled as a loadable module, and etnaviv is
> built-in, we get a link error:
>
> drivers/gpu/drm/etnaviv/etnaviv_gpu.o: In function `etnaviv_gpu_bind':
> etnaviv_gpu.c:(.text.etnaviv_gpu_bind+0x34): undefined reference to
> `thermal_of_cooling_device_register'
> etnaviv_gpu.c:(.text.etnaviv_gpu_bind+0xe0): undefined reference to
> `thermal_cooling_device_unregister'
> drivers/gpu/drm/etnaviv/etnaviv_gpu.o: In function `etnaviv_gpu_unbind':
> etnaviv_gpu.c:(.text.etnaviv_gpu_unbind+0xf0): undefined reference to
> `thermal_cooling_device_unregister'
>
> This adds a Kconfig dependency to prevent etnaviv from being built-in
> with CONFIG_THERMAL=m, while still allowing the valid configurations.
> Unfortunately, simply adding the dependency here breaks Kconfig through
> a dependency loop involving lots of symbols all the way until ACPI_VIDEO,
> which is the only video driver that explicitly selects 'THERMAL'. Turning
> this into a 'depends on' addresses the problem.
> For completeness, I'm also removing the redundant 'select THERMAL'
> from INTEL_MENLOW, so no other driver uses that statement.
>
> Fixes: bcdfb5e56dc5 ("drm/etnaviv: add etnaviv cooling device")
> Signed-off-by: Arnd Bergmann
> ---
> v2: spend more thought on ACPI_VIDEO dependencies, as we need another
> patch before this.
> ---
> drivers/acpi/Kconfig| 2 +-
> drivers/gpu/drm/etnaviv/Kconfig | 1 +
> drivers/platform/x86/Kconfig| 1 -
I would like to take this patch, but I'm not sure why it includes x86
and ACPI hunks.
Regards,
Lucas
> 3 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index a8f5a40e2914..1282b2936164 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -184,7 +184,7 @@ config ACPI_VIDEO
> tristate "Video"
> depends on X86
> depends on INPUT
> - select THERMAL
> + depends on THERMAL
> select BACKLIGHT_CLASS_DEVICE
> select BACKLIGHT_LCD_SUPPORT
> default y
> diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
> index 71cee4e9fefb..1d6c744e7924 100644
> --- a/drivers/gpu/drm/etnaviv/Kconfig
> +++ b/drivers/gpu/drm/etnaviv/Kconfig
> @@ -3,6 +3,7 @@ config DRM_ETNAVIV
> tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
> depends on DRM
> depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
> + depends on THERMAL || THERMAL=n
> depends on MMU
> select SHMEM
> select SYNC_FILE
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 2d9fb46a8d11..d9238e9ff54a 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -532,7 +532,6 @@ config SENSORS_HDAPS
> config INTEL_MENLOW
> tristate "Thermal Management driver for Intel menlow platform"
> depends on ACPI_THERMAL
> - select THERMAL
> ---help---
> ACPI thermal management enhancement driver on
> Intel Menlow platform.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/3] imx-drm: Add mx6 hdmi transmitter support
Hi Russell, Am Mittwoch, den 16.10.2013, 20:02 +0100 schrieb Russell King - ARM Linux: > On Wed, Oct 16, 2013 at 11:31:07AM -0700, Greg Kroah-Hartman wrote: > > On Wed, Oct 16, 2013 at 07:07:35PM +0100, Russell King - ARM Linux wrote: > > > Sorry, but I don't think imx-drm is driving the hardware correctly, and > > > I know that Greg wants it moved out of drivers/staging, but frankly it > > > seems to be far from ready for that. Certainly the HDMI parts seems to > > > be especially problematical. > > > > I want it out of staging if it's working properly. Yours is the first > > report of it not working properly, and in fact, probably one of the > > first users of the driver, as I haven't gotten any reports of it working > > or not at all over the years. > > Well, part of that is because I have this other thing called Armada DRM, > which is a similar thing to imx-drm, except for the Marvell Dove SoCs, > so it's been really quite easy to get a full Ubuntu 12.04 up and running > on the IMX SoC I have here. > > As part of that effort, I'm now using my Armada DRM Xorg driver with > imx-drm to test this stuff out. (Bearing in mind that IMX and Dove use > the same Vivante GPU, there's some sense in getting my Xorg Armada DRM > driver working on both.) > > I'm not aware of there being any X drivers for imx-drm (google turns > up nothing), which might be a reason why it hasn't been well tested > and has also languished in drivers/staging for soo long. Alternatively, > maybe my google searching sucks, or it is out there somewhere but > hidden from googlebot? > X.Org isn't the center of the world anymore. For some general purpose distros this might still hold true for some time, but certainly not for the use-cases we test this driver with. For most of our embedded use-cases we currently use the FBdev emulation (phasing this out at the moment) or the apps are sitting right on top of the raw KMS APIs, rather than on top of a display server. We also did some experiments with Wayland (Weston) on top of imx-drm, which worked quite well. There might be dragons hiding in some corners for the more general purpose use-cases and we are happy to have you test the driver and provide valuable reports. > To be fair, so far most of the problems I'm finding are with the HDMI > addition to imx-drm rather than imx-drm itself: there's been the lockdep > problem and the clock polarity problem which the HDMI stuff discovered. > > Looking at the todo list for moving it out of staging, we have: > > - get DRM Maintainer review for this code > - Wait for common display framework to hit mainline and update the IPU > driver to use it. This will most probably make changes to the devicetree > bindings necessary. > - 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 > > (1) is quite a difficult task: I'm waiting for a review of the Armada DRM > stuff, which I'm hoping will make the next merge window. Rob Clark is > looking at that but has been distracted recently from completing it. > > (2) CDF... has been around in RFC according to LWN.net but doesn't seem to > make much in the way of progress from what I can see, despite there > allegedly being "many developers show[ing] interest in the first RFC". > CDF is over a year old now - first RFC 17 Aug 2012, second 22 Nov 2012, > third 9 Aug 2013. > Philipp has a prototype of CDF on imx-drm working internally and I suspect he will be able to post patches shortly after ELCE. > (3) is an on-going effort and really isn't a reason for it to stay in > staging. > > (4) is probably the biggest issue. The problem there is that the IMX > display subsystem is very flexible: it's basically a set of DMA engines > on the front, and behind that a fair number of modules implementing > facilities like image rotation, display driving and image capture. > Display driving alone involves setting up waveforms and writing some > microcode to tell the thing what to do on certain events (like end > of frame etc.) Hence it doesn't fit well into any particular "Linux" > subsystem. In this regard, it's a victim of its own flexibility. We are aiming to do the same thing as the Tegra host1x driver: move the IPU core to drivers/gpu and export API for other drivers to use. This may mean we still have to keep the DRM part in staging (at least until the CDF thing is sorted, as this prevents us from setting the devicetree bindings in stone), but at least should be a step in the right direction. Regards, Lucas -- Pengutronix e.K.
Re: [PATCHv5][ 2/8] staging: imx-drm: Add RGB666 support for parallel display.
Am Freitag, den 06.12.2013, 14:14 +0100 schrieb Thierry Reding: > On Thu, Dec 05, 2013 at 07:28:06PM +0100, Denis Carikli wrote: > [...] > > diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c > > b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c > [...] > > @@ -155,6 +156,8 @@ static int ipu_pixfmt_to_map(u32 fmt) > > return IPU_DC_MAP_BGR666; > > case V4L2_PIX_FMT_BGR24: > > return IPU_DC_MAP_BGR24; > > + case V4L2_PIX_FMT_RGB666: > > + return IPU_DC_MAP_RGB666; > > Why is this DRM driver even using V4L2 pixel formats in the first place? > Because imx-drm is actually a misnomer. The i.MX IPU is a multifunction device, which as one part has the display controllers, but also camera interfaces and mem-to-mem scaler devices, which are hooked up via the V4L2 interface. The generic IPU part, which is used for example for programming the DMA channels is using V4L2 pixel formats as a common base. We have patches to split this out and make this fact more visible. (The IPU core will be placed aside the Tegra host1x driver) Regards, Lucas -- Pengutronix e.K. | Lucas Stach | Industrial Linux Solutions | http://www.pengutronix.de/ | Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-5076 | Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- | ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 1/3] staging: drm/imx: set second plane base address
From: Philipp Zabel
Even though we do not enable the hardware double buffering feature
right now, set the second base address pointer (EBA1) as well to
increase robustness.
Signed-off-by: Philipp Zabel
Signed-off-by: Lucas Stach
---
drivers/staging/imx-drm/ipuv3-plane.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c
b/drivers/staging/imx-drm/ipuv3-plane.c
index d97454a0dffd..685411ce6e39 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.c
+++ b/drivers/staging/imx-drm/ipuv3-plane.c
@@ -64,6 +64,7 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct
drm_framebuffer *fb,
{
struct ipu_ch_param __iomem *cpmem;
struct drm_gem_cma_object *cma_obj;
+ unsigned long eba;
cma_obj = drm_fb_cma_get_gem_obj(fb, 0);
if (!cma_obj) {
@@ -76,8 +77,10 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct
drm_framebuffer *fb,
cpmem = ipu_get_cpmem(ipu_plane->ipu_ch);
ipu_cpmem_set_stride(cpmem, fb->pitches[0]);
- ipu_cpmem_set_buffer(cpmem, 0, cma_obj->paddr + fb->offsets[0] +
-fb->pitches[0] * y + x);
+
+ eba = cma_obj->paddr + fb->offsets[0] + fb->pitches[0] * y + x;
+ ipu_cpmem_set_buffer(cpmem, 0, eba);
+ ipu_cpmem_set_buffer(cpmem, 1, eba);
return 0;
}
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 0/3] imx-drm crtc/plane offset fixes
These patches fix the issue of frame buffer offsets being forgotten during crtc pageflip. Also we set the second buffer address. v2: proper changelogs, no functional change Lucas Stach (2): staging: drm/imx: handle framebuffer offsets correctly staging: drm/imx: don't drop crtc offsets when doing pageflip Philipp Zabel (1): staging: drm/imx: set second plane base address drivers/staging/imx-drm/ipuv3-crtc.c | 3 ++- drivers/staging/imx-drm/ipuv3-plane.c | 12 ++-- 2 files changed, 12 insertions(+), 3 deletions(-) -- 1.8.5.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 3/3] staging: drm/imx: don't drop crtc offsets when doing pageflip
CRTC offsets are only set with the initial modeset, any
subseqent pageflips assume them to be kept the same, so
we need to remember the current state until another modeset
changes it.
Signed-off-by: Philipp Zabel
Signed-off-by: Lucas Stach
---
drivers/staging/imx-drm/ipuv3-crtc.c | 3 ++-
drivers/staging/imx-drm/ipuv3-plane.c | 4
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c
b/drivers/staging/imx-drm/ipuv3-crtc.c
index ce6ba987ec91..22be104fbda9 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -218,7 +218,8 @@ static irqreturn_t ipu_irq_handler(int irq, void *dev_id)
if (ipu_crtc->newfb) {
ipu_crtc->newfb = NULL;
- ipu_plane_set_base(ipu_crtc->plane[0], ipu_crtc->base.fb, 0, 0);
+ ipu_plane_set_base(ipu_crtc->plane[0], ipu_crtc->base.fb,
+ ipu_crtc->plane[0]->x, ipu_crtc->plane[0]->y);
ipu_crtc_handle_pageflip(ipu_crtc);
}
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c
b/drivers/staging/imx-drm/ipuv3-plane.c
index 2ef95162b3e9..34b642a12f8b 100644
--- a/drivers/staging/imx-drm/ipuv3-plane.c
+++ b/drivers/staging/imx-drm/ipuv3-plane.c
@@ -83,6 +83,10 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct
drm_framebuffer *fb,
ipu_cpmem_set_buffer(cpmem, 0, eba);
ipu_cpmem_set_buffer(cpmem, 1, eba);
+ /* cache offsets for subsequent pageflips */
+ ipu_plane->x = x;
+ ipu_plane->y = y;
+
return 0;
}
--
1.8.5.2
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2 2/3] staging: drm/imx: handle framebuffer offsets correctly
We didn't take the pixel format into account, so x-direction offsets were off by a factor of 2 or 4 for 16bpp and 32bpp framebuffers. Signed-off-by: Philipp Zabel Signed-off-by: Lucas Stach --- drivers/staging/imx-drm/ipuv3-plane.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c index 685411ce6e39..2ef95162b3e9 100644 --- a/drivers/staging/imx-drm/ipuv3-plane.c +++ b/drivers/staging/imx-drm/ipuv3-plane.c @@ -78,7 +78,8 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb, cpmem = ipu_get_cpmem(ipu_plane->ipu_ch); ipu_cpmem_set_stride(cpmem, fb->pitches[0]); - eba = cma_obj->paddr + fb->offsets[0] + fb->pitches[0] * y + x; + eba = cma_obj->paddr + fb->offsets[0] + + fb->pitches[0] * y + (fb->bits_per_pixel >> 3) * x; ipu_cpmem_set_buffer(cpmem, 0, eba); ipu_cpmem_set_buffer(cpmem, 1, eba); -- 1.8.5.2 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
