Re: [PATCHv7][ 4/7] staging: imx-drm: Use de-active and pixelclk-active display-timings.

2014-02-21 Thread Lothar Waßmann
Hi,

Denis Carikli wrote:
> If de-active and/or pixelclk-active properties were set in the
> display-timings DT node, they were not used.
> 
> Instead the data-enable and the pixel data clock polarity
> were hardcoded.
> 
> This change is needed for making the eukrea-cpuimx51
>   QVGA display work.
> 
> Cc: Eric Bénard 
> Cc: Greg Kroah-Hartman 
> Cc: Sascha Hauer 
> Cc: de...@driverdev.osuosl.org
> Cc: linux-arm-ker...@lists.infradead.org
> Signed-off-by: Denis Carikli 
> ---
> ChangeLog v6->v7:
> - Shrinked even more the Cc list.
> - Rebased the patch
> - val is now initialized in imx_pd_connector_get_modes
> 
> ChangeLog v5->v6:
> - Remove people not concerned by this patch from the Cc list.
> - Removed wrong coments from the code.
> - Corrected the code style of the "if (!!val)"
> 
> ChangeLog v3->v4:
> - The old patch was named "staging: imx-drm: ipuv3-crtc: don't harcode some 
> mode".
> - Reworked the patch entierly: we now takes the mode flags from the device 
> tree.
> 
> ChangeLog v2->v3:
> - Added some interested people in the Cc list.
> - Ajusted the flags to match the changes in "drm: Add the lacking
>   DRM_MODE_FLAG_* for matching the DISPLAY_FLAGS_*"
> ---
>  drivers/staging/imx-drm/imx-drm.h  |3 +++
>  drivers/staging/imx-drm/ipuv3-crtc.c   |8 ++--
>  drivers/staging/imx-drm/parallel-display.c |   27 +++
>  3 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/imx-drm/imx-drm.h 
> b/drivers/staging/imx-drm/imx-drm.h
> index ae90c9c..dfdc180 100644
> --- a/drivers/staging/imx-drm/imx-drm.h
> +++ b/drivers/staging/imx-drm/imx-drm.h
> @@ -5,6 +5,9 @@
>  
>  #define IPU_PIX_FMT_GBR24v4l2_fourcc('G', 'B', 'R', '3')
>  
> +#define IMXDRM_MODE_FLAG_DE_HIGH (1<<0)
> +#define IMXDRM_MODE_FLAG_PIXDATA_POSEDGE (1<<1)
> +
CodingStyle: spaces around operators...?


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv8][ 4/7] staging: imx-drm: Use de-active and pixelclk-active display-timings.

2014-03-05 Thread Lothar Waßmann
Hi,

Denis Carikli wrote:
> If de-active and/or pixelclk-active properties were set in the
> display-timings DT node, they were not used.
> 
> Instead the data-enable and the pixel data clock polarity
> were hardcoded.
> 
> This change is needed for making the eukrea-cpuimx51
>   QVGA display work.
> 
I just tried this patch on our hardware and found that the pixelclock
polarity is inverse to what the documentation says.

Your patch sets the 'clk_pol' variable in positive logic, while it is
interpreted in negative logic when converted to the final register
value in drivers/staging/imx-drm/ipu-v3/ipu-di.c:

if (!(sig->clk_pol))
di_gen |= DI_GEN_POLARITY_DISP_CLK;

IMO this should be
if (sig->clk_pol)
di_gen |= DI_GEN_POLARITY_DISP_CLK;

Did you actually measure the resulting clock signal and LCD data?


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v9][ 6/8] staging: imx-drm: parallel display: add regulator support.

2014-03-07 Thread Lothar Waßmann
Hi,

Denis Carikli wrote:
> Signed-off-by: Denis Carikli 
> ---
> ChangeLog v8->v9:
> - Removed the Cc. They are now set in git-send-email directly.
> - Rebased.
> 
> ChangeLog v7->v8:
> - Shrinked even more the Cc list.
> - Rebased.
> 
> ChangeLog v6->v7:
> - Shrinked even more the Cc list.
> - Rebased the patch and included video/of_display_timing.h
> ---
>  .../bindings/staging/imx-drm/fsl-imx-drm.txt   |1 +
>  drivers/staging/imx-drm/parallel-display.c |   13 +
>  2 files changed, 14 insertions(+)
> 
> diff --git 
> a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt 
> b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
> index 2d24425..4dd7ce5 100644
> --- a/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
> +++ b/Documentation/devicetree/bindings/staging/imx-drm/fsl-imx-drm.txt
> @@ -28,6 +28,7 @@ Required properties:
>  - compatible: Should be "fsl,imx-parallel-display"
>  - crtc: the crtc this display is connected to, see below
>  Optional properties:
> +- display-supply : phandle to the regulator device tree node if needed.
>
Any reason why this is named 'display-supply' rather than 'lcd-supply'
like for imxfb?


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH v9][ 3/8] staging: imx-drm: Correct BGR666 and the board's dts that use them.

2014-03-12 Thread Lothar Waßmann
Hi,

Denis Carikli wrote:
> The current BGR666 is not consistent with the other color mapings like BGR24.
> BGR666 should be in the same byte order than BGR24.
> 
[...]
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c 
> b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
> index 6f9abe8..154d293 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-dc.c
> @@ -397,9 +397,9 @@ int ipu_dc_init(struct ipu_soc *ipu, struct device *dev,
>  
>   /* bgr666 */
>   ipu_dc_map_clear(priv, IPU_DC_MAP_BGR666);
> - ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 5, 0xfc); /* blue */
> + ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 0, 17, 0xfc); /* blue */
>   ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 1, 11, 0xfc); /* green */
> - ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 17, 0xfc); /* red */
> + ipu_dc_map_config(priv, IPU_DC_MAP_BGR666, 2, 5, 0xfc); /* red */
>  
>   /* bgr24 */
>   ipu_dc_map_clear(priv, IPU_DC_MAP_BGR24);
>
You obviously missed this one which is also affected by your change:
diff --git a/drivers/staging/imx-drm/imx-ldb.c 
b/drivers/staging/imx-drm/imx-ldb.c
index daa54df..e5a600b 100644
--- a/drivers/staging/imx-drm/imx-ldb.c
+++ b/drivers/staging/imx-drm/imx-ldb.c
@@ -185,11 +185,11 @@ static void imx_ldb_encoder_prepare(struct drm_encoder 
*encoder)
switch (imx_ldb_ch->chno) {
case 0:
pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH0_24) ?
-   V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+   V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_RGB666;
break;
case 1:
pixel_fmt = (ldb->ldb_ctrl & LDB_DATA_WIDTH_CH1_24) ?
-   V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_BGR666;
+   V4L2_PIX_FMT_RGB24 : V4L2_PIX_FMT_RGB666;
break;
default:
dev_err(ldb->dev, "unable to config di%d panel format\n",

Without this patch Red/Blue on an 18bit LVDS display will be swapped.


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 07/12] drm: drm_display_mode: add signal polarity flags

2014-03-17 Thread Lothar Waßmann
Hi,

Laurent Pinchart wrote:
> Hello,
> 
> On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote:
> > On 03/13/2014 06:17 PM, Denis Carikli wrote:
> > > We need a way to pass signal polarity informations
> > > between DRM panels, and the display drivers.
> > > 
> > > To do that, a pol_flags field was added to drm_display_mode.
> > > 
> > > Signed-off-by: Denis Carikli 
> > > ---
> > > ChangeLog v10->v11:
> > > - Since the imx-drm won't be able to retrive its regulators
> > > 
> > >   from the device tree when using display-timings nodes,
> > >   and that I was told that the drm simple-panel driver
> > >   already supported that, I then, instead, added what was
> > >   lacking to make the eukrea displays work with the
> > >   drm-simple-panel driver.
> > >   
> > >   That required a way to get back the display polarity
> > >   informations from the imx-drm driver without affecting
> > >   userspace.
> > > 
> > > ---
> > > 
> > >  include/drm/drm_crtc.h |8 
> > >  1 file changed, 8 insertions(+)
> > > 
> > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > index f764654..61a4fe1 100644
> > > --- a/include/drm/drm_crtc.h
> > > +++ b/include/drm/drm_crtc.h
> > > @@ -131,6 +131,13 @@ enum drm_mode_status {
> > > 
> > >  #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
> > > 
> > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGEBIT(1)
> > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGEBIT(2)
> > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE   BIT(3)
> > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4)
> > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5)
> > > +#define DRM_MODE_FLAG_POL_DE_PRESERVEBIT(6)
> > 
> > Could you add some description to these flags.
> > What are *_PRESERVE flags for?
> > Are those flags 1:1 compatible with respective 'videomode:flags'?
> > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), am I
> > right?
> 
> Possibly nitpicking, I wouldn't call the clock edge on which data signals are 
> generated/sampled "data polarity". This is clock polarity information.
> 
> Have you seen cases where pixel data and DE are geenrated or need to be 
> sampled on different edges ?
> 
DE is not a clock signal, but an 'Enable' signal whose value (high or
low) defines the window in which the pixel data is valid.
The flag defines whether data is valid during the HIGH or LOW period of
DE.


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 07/12] drm: drm_display_mode: add signal polarity flags

2014-03-18 Thread Lothar Waßmann
Hi,

Laurent Pinchart wrote:
> Hi Lothar,
> 
> On Monday 17 March 2014 16:14:36 Lothar Waßmann wrote:
> > Laurent Pinchart wrote:
> > > On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote:
> > > > On 03/13/2014 06:17 PM, Denis Carikli wrote:
> > > > > We need a way to pass signal polarity informations
> > > > > between DRM panels, and the display drivers.
> > > > > 
> > > > > To do that, a pol_flags field was added to drm_display_mode.
> > > > > 
> > > > > Signed-off-by: Denis Carikli 
> > > > > ---
> > > > > ChangeLog v10->v11:
> > > > > - Since the imx-drm won't be able to retrive its regulators
> > > > > 
> > > > >   from the device tree when using display-timings nodes,
> > > > >   and that I was told that the drm simple-panel driver
> > > > >   already supported that, I then, instead, added what was
> > > > >   lacking to make the eukrea displays work with the
> > > > >   drm-simple-panel driver.
> > > > >   
> > > > >   That required a way to get back the display polarity
> > > > >   informations from the imx-drm driver without affecting
> > > > >   userspace.
> > > > > 
> > > > > ---
> > > > > 
> > > > >  include/drm/drm_crtc.h |8 
> > > > >  1 file changed, 8 insertions(+)
> > > > > 
> > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > > > index f764654..61a4fe1 100644
> > > > > --- a/include/drm/drm_crtc.h
> > > > > +++ b/include/drm/drm_crtc.h
> > > > > @@ -131,6 +131,13 @@ enum drm_mode_status {
> > > > > 
> > > > >  #define DRM_MODE_FLAG_3D_MAX DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
> > > > > 
> > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGEBIT(1)
> > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGEBIT(2)
> > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE   BIT(3)
> > > > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4)
> > > > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5)
> > > > > +#define DRM_MODE_FLAG_POL_DE_PRESERVEBIT(6)
> > > > 
> > > > Could you add some description to these flags.
> > > > What are *_PRESERVE flags for?
> > > > Are those flags 1:1 compatible with respective 'videomode:flags'?
> > > > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH), am I
> > > > right?
> > > 
> > > Possibly nitpicking, I wouldn't call the clock edge on which data signals
> > > are generated/sampled "data polarity". This is clock polarity
> > > information.
> > > 
> > > Have you seen cases where pixel data and DE are geenrated or need to be
> > > sampled on different edges ?
> > 
> > DE is not a clock signal, but an 'Enable' signal whose value (high or
> > low) defines the window in which the pixel data is valid.
> > The flag defines whether data is valid during the HIGH or LOW period of
> > DE.
> 
> The DRM_MODE_FLAG_POL_DE_(LOW|HIGH) do, by my impression of the proposed new 
> DRM_MODE_FLAG_POL_DE_*EDGE flags is that they define sampling clock edges, 
> not 
> active levels.
> 
The current naming of the flags gives the impression that they describe
the sampling edges of a clock signal. But the DE signal in fact is not
a clock signal but a level sensitive gating signal.


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 07/12] drm: drm_display_mode: add signal polarity flags

2014-03-18 Thread Lothar Waßmann
Hi,

Laurent Pinchart wrote:
> Hi Lothar,
> 
> On Tuesday 18 March 2014 08:50:30 Lothar Waßmann wrote:
> > Laurent Pinchart wrote:
> > > On Monday 17 March 2014 16:14:36 Lothar Waßmann wrote:
> > > > Laurent Pinchart wrote:
> > > > > On Monday 17 March 2014 14:41:09 Andrzej Hajda wrote:
> > > > > > On 03/13/2014 06:17 PM, Denis Carikli wrote:
> > > > > > > We need a way to pass signal polarity informations
> > > > > > > between DRM panels, and the display drivers.
> > > > > > > 
> > > > > > > To do that, a pol_flags field was added to drm_display_mode.
> > > > > > > 
> > > > > > > Signed-off-by: Denis Carikli 
> > > > > > > ---
> > > > > > > ChangeLog v10->v11:
> > > > > > > - Since the imx-drm won't be able to retrive its regulators
> > > > > > > 
> > > > > > >   from the device tree when using display-timings nodes,
> > > > > > >   and that I was told that the drm simple-panel driver
> > > > > > >   already supported that, I then, instead, added what was
> > > > > > >   lacking to make the eukrea displays work with the
> > > > > > >   drm-simple-panel driver.
> > > > > > >   
> > > > > > >   That required a way to get back the display polarity
> > > > > > >   informations from the imx-drm driver without affecting
> > > > > > >   userspace.
> > > > > > > 
> > > > > > > ---
> > > > > > > 
> > > > > > >  include/drm/drm_crtc.h |8 
> > > > > > >  1 file changed, 8 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> > > > > > > index f764654..61a4fe1 100644
> > > > > > > --- a/include/drm/drm_crtc.h
> > > > > > > +++ b/include/drm/drm_crtc.h
> > > > > > > @@ -131,6 +131,13 @@ enum drm_mode_status {
> > > > > > > 
> > > > > > >  #define DRM_MODE_FLAG_3D_MAX 
> > > > > > > DRM_MODE_FLAG_3D_SIDE_BY_SIDE_HALF
> > > > > > > 
> > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_NEGEDGEBIT(1)
> > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_POSEDGEBIT(2)
> > > > > > > +#define DRM_MODE_FLAG_POL_PIXDATA_PRESERVE   BIT(3)
> > > > > > > +#define DRM_MODE_FLAG_POL_DE_NEGEDGE BIT(4)
> > > > > > > +#define DRM_MODE_FLAG_POL_DE_POSEDGE BIT(5)
> > > > > > > +#define DRM_MODE_FLAG_POL_DE_PRESERVEBIT(6)
> > > > > > 
> > > > > > Could you add some description to these flags.
> > > > > > What are *_PRESERVE flags for?
> > > > > > Are those flags 1:1 compatible with respective 'videomode:flags'?
> > > > > > I guess DE flags should be rather DRM_MODE_FLAG_POL_DE_(LOW|HIGH),
> > > > > > am I right?
> > > > > 
> > > > > Possibly nitpicking, I wouldn't call the clock edge on which data
> > > > > signals are generated/sampled "data polarity". This is clock polarity
> > > > > information.
> > > > > 
> > > > > Have you seen cases where pixel data and DE are geenrated or need to
> > > > > be sampled on different edges ?
> > > > 
> > > > DE is not a clock signal, but an 'Enable' signal whose value (high or
> > > > low) defines the window in which the pixel data is valid.
> > > > The flag defines whether data is valid during the HIGH or LOW period of
> > > > DE.
> > > 
> > > The DRM_MODE_FLAG_POL_DE_(LOW|HIGH) do, by my impression of the proposed
> > > new DRM_MODE_FLAG_POL_DE_*EDGE flags is that they define sampling clock
> > > edges, not active levels.
> > 
> > The current naming of the flags gives the impression that they describe
> > the sampling edges of a clock signal. But the DE signal in fact is not
> > a clock signal but a level sensitive gating signal.
> 
> That's not my point. I *know* that DE is a data gating signal with a polarity 
> already defined by the DRM_MODE_FLAG_POL_DE_(LOW|HIGH) flags. Like all other 
> signals it gets generated on a clock edge and is sampled on a clock edge. The 
> DRM_MODE_FLAG_POL_DE_*EDGE flags proposed above describe seem to describe 
> just 
>
The important word here is 'seem'.


Lothar Waßann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 08/12] imx-drm: Use drm_display_mode timings flags.

2014-03-18 Thread Lothar Waßmann
Hi,

Denis Carikli wrote:
> The previous hardware behaviour was kept if the
> flags are not set.
> 
> Signed-off-by: Denis Carikli 
> ---
> ChangeLog v10->v11:
> - This patch was splitted-out and adapted from:
>   "Prepare imx-drm for extra display-timings retrival."
> - The display-timings dt specific part was removed.
> - The flags names were changed to use the DRM ones from:
>   "drm: drm_display_mode: add signal polarity flags"
> ---
>  drivers/staging/imx-drm/imx-drm-core.c  |   10 ++
>  drivers/staging/imx-drm/imx-drm.h   |6 ++
>  drivers/staging/imx-drm/imx-hdmi.c  |3 +++
>  drivers/staging/imx-drm/imx-ldb.c   |3 +++
>  drivers/staging/imx-drm/imx-tve.c   |3 +++
>  drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h |6 --
>  drivers/staging/imx-drm/ipu-v3/ipu-di.c |7 ++-
>  drivers/staging/imx-drm/ipuv3-crtc.c|   21 +++--
>  3 files changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h 
> b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
> index b95cba1..3abeea3 100644
> --- a/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
> +++ b/drivers/staging/imx-drm/ipu-v3/imx-ipu-v3.h
> @@ -29,9 +29,11 @@ enum ipuv3_type {
>  
>  #define CLK_POL_ACTIVE_LOW   0
>  #define CLK_POL_ACTIVE_HIGH  1
> +#define CLK_POL_PRESERVE 2
>  
>  #define ENABLE_POL_NEGEDGE   0
>  #define ENABLE_POL_POSEDGE   1
> +#define ENABLE_POL_PRESERVE  2
>  
>  /*
>   * Bitfield of Display Interface signal polarities.
> @@ -43,10 +45,10 @@ struct ipu_di_signal_cfg {
>   unsigned clksel_en:1;
>   unsigned clkidle_en:1;
>   unsigned data_pol:1;/* true = inverted */
> - unsigned clk_pol:1;
> - unsigned enable_pol:1;
>   unsigned Hsync_pol:1;   /* true = active high */
>   unsigned Vsync_pol:1;
> + u8 clk_pol;
> + u8 enable_pol;
>  
>   u16 width;
>   u16 height;
> diff --git a/drivers/staging/imx-drm/ipu-v3/ipu-di.c 
> b/drivers/staging/imx-drm/ipu-v3/ipu-di.c
> index 53646aa..791080b 100644
> --- a/drivers/staging/imx-drm/ipu-v3/ipu-di.c
> +++ b/drivers/staging/imx-drm/ipu-v3/ipu-di.c
> @@ -597,6 +597,8 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct 
> ipu_di_signal_cfg *sig)
>  
>   if (sig->clk_pol == CLK_POL_ACTIVE_HIGH)
>   di_gen |= DI_GEN_POLARITY_DISP_CLK;
> + else if (sig->clk_pol == CLK_POL_ACTIVE_LOW)
> + di_gen &= ~DI_GEN_POLARITY_DISP_CLK;
>  
>   ipu_di_write(di, di_gen, DI_GENERAL);
>  
> @@ -604,10 +606,13 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct 
> ipu_di_signal_cfg *sig)
>DI_SYNC_AS_GEN);
>  
>   reg = ipu_di_read(di, DI_POL);
> - reg &= ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
> + reg &= ~(DI_POL_DRDY_DATA_POLARITY);
>  
>   if (sig->enable_pol == ENABLE_POL_POSEDGE)
>   reg |= DI_POL_DRDY_POLARITY_15;
> + else if (sig->enable_pol == ENABLE_POL_NEGEDGE)
> + reg &= ~DI_POL_DRDY_POLARITY_15;
> +
>   if (sig->data_pol)
>   reg |= DI_POL_DRDY_DATA_POLARITY;
>  
> diff --git a/drivers/staging/imx-drm/ipuv3-crtc.c 
> b/drivers/staging/imx-drm/ipuv3-crtc.c
> index 8cfeb47..c75034e 100644
> --- a/drivers/staging/imx-drm/ipuv3-crtc.c
> +++ b/drivers/staging/imx-drm/ipuv3-crtc.c
> @@ -157,8 +157,25 @@ static int ipu_crtc_mode_set(struct drm_crtc *crtc,
>   if (mode->flags & DRM_MODE_FLAG_PVSYNC)
>   sig_cfg.Vsync_pol = 1;
>  
> - sig_cfg.enable_pol = ENABLE_POL_POSEDGE;
> - sig_cfg.clk_pol = CLK_POL_ACTIVE_LOW;
> + if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_POSEDGE)
> + sig_cfg.enable_pol = ENABLE_POL_POSEDGE;
> + else if (mode->pol_flags & DRM_MODE_FLAG_POL_DE_NEGEDGE)
> + sig_cfg.enable_pol = ENABLE_POL_NEGEDGE;
> + else if (mode->pol_flags & DRM_MODE_FLAG_POL_PIXDATA_PRESERVE)
> + sig_cfg.enable_pol = ENABLE_POL_PRESERVE;
> + else
> + sig_cfg.enable_pol = ENABLE_POL_POSEDGE;
> +
> + if (mode->private_flags & DRM_MODE_FLAG_POL_DE_POSEDGE)
> + sig_cfg.clk_pol = CLK_POL_ACTIVE_HIGH;
> + else if (mode->private_flags & DRM_MODE_FLAG_POL_DE_NEGEDGE)
> + sig_cfg.clk_pol = CLK_POL_ACTIVE_LOW;
> +     else if (mode->private_flags & DRM_MODE_FLAG_POL_DE_PRESERVE)
> + sig_cfg.clk_pol = CLK_POL_PRESERVE;
> + else
> + sig_cfg.clk_pol = CLK_POL_ACTIVE_LOW;
>
This is completely messed up. POL_PIXDATA should obviously 

Re: [PATCH v12][ 06/12] ARM: dts: imx5*, imx6*: correct display-timings nodes.

2014-04-08 Thread Lothar Waßmann
Hi,

Shawn Guo wrote:
> On Mon, Apr 07, 2014 at 02:44:45PM +0200, Denis Carikli wrote:
> > The imx-drm driver can't use the de-active and
> > pixelclk-active display-timings properties yet.
> > 
> > Instead the data-enable and the pixel data clock
> > polarity are hardcoded in the imx-drm driver.
> > 
> > So theses properties are now set to keep
> > the same behaviour when imx-drm will start
> > using them.
> > 
> > Signed-off-by: Denis Carikli 
> > ---
> > ChangeLog v9->v10:
> > - New patch that was splitted out of:
> >   "staging imx-drm: Use de-active and pixelclk-active
> >   display-timings."
> > ---
> >  arch/arm/boot/dts/imx51-babbage.dts   |2 ++
> >  arch/arm/boot/dts/imx53-m53evk.dts|2 ++
> >  arch/arm/boot/dts/imx53-tx53-x03x.dts |2 +-
> >  arch/arm/boot/dts/imx6qdl-gw53xx.dtsi |2 ++
> >  arch/arm/boot/dts/imx6qdl-gw54xx.dtsi |2 ++
> >  arch/arm/boot/dts/imx6qdl-nitrogen6x.dtsi |2 ++
> >  arch/arm/boot/dts/imx6qdl-sabreauto.dtsi  |2 ++
> >  arch/arm/boot/dts/imx6qdl-sabrelite.dtsi  |2 ++
> >  arch/arm/boot/dts/imx6qdl-sabresd.dtsi|2 ++
> >  9 files changed, 17 insertions(+), 1 deletion(-)
> 
> ...
> 
> > diff --git a/arch/arm/boot/dts/imx53-tx53-x03x.dts 
> > b/arch/arm/boot/dts/imx53-tx53-x03x.dts
> > index 0217dde3..4092a81 100644
> > --- a/arch/arm/boot/dts/imx53-tx53-x03x.dts
> > +++ b/arch/arm/boot/dts/imx53-tx53-x03x.dts
> > @@ -93,7 +93,7 @@
> > hsync-active = <0>;
> > vsync-active = <0>;
> > de-active = <1>;
> > -   pixelclk-active = <1>;
> > +   pixelclk-active = <0>;
> 
> @Lothar, is this change correct?
> 
No, the ET0430 display which is affected by this patch actually has an
inverted clock wrt the other displays of the family.

'pixelclk-active = <1>' is the correct setting for this display!

Thanks, Shawn for the reminder.


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 10/20] gpio: pca953x: Add optional reset gpio control

2016-12-29 Thread Lothar Waßmann
Hi,

On Thu, 29 Dec 2016 14:27:25 -0800 Steve Longerbeam wrote:
> Add optional reset-gpios pin control. If present, de-assert the
> specified reset gpio pin to bring the chip out of reset.
> 
> Signed-off-by: Steve Longerbeam 
> Cc: Linus Walleij 
> Cc: Alexandre Courbot 
> Cc: linux-g...@vger.kernel.org
> Cc: linux-ker...@vger.kernel.org
> 
> ---
> 
> v2:
> - documented optional reset-gpios property in
>   Documentation/devicetree/bindings/gpio/gpio-pca953x.txt.
> ---
>  Documentation/devicetree/bindings/gpio/gpio-pca953x.txt |  4 
>  drivers/gpio/gpio-pca953x.c | 17 
> +
>  2 files changed, 21 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt 
> b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> index 08dd15f..da54f4c 100644
> --- a/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> +++ b/Documentation/devicetree/bindings/gpio/gpio-pca953x.txt
> @@ -29,6 +29,10 @@ Required properties:
>   onsemi,pca9654
>   exar,xra1202
>  
> +Optional properties:
> + - reset-gpios: GPIO specification for the RESET input
> +
> +
>  Example:
>  
>  
> diff --git a/drivers/gpio/gpio-pca953x.c b/drivers/gpio/gpio-pca953x.c
> index d5d72d8..d1c0bd5 100644
> --- a/drivers/gpio/gpio-pca953x.c
> +++ b/drivers/gpio/gpio-pca953x.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #define PCA953X_INPUT0
>  #define PCA953X_OUTPUT   1
> @@ -133,6 +134,7 @@ struct pca953x_chip {
>   const char *const *names;
>   unsigned long driver_data;
>   struct regulator *regulator;
> + struct gpio_desc *reset_gpio;
>  
>   const struct pca953x_reg_config *regs;
>  
> @@ -756,6 +758,21 @@ static int pca953x_probe(struct i2c_client *client,
>   } else {
>   chip->gpio_start = -1;
>   irq_base = 0;
> +
> + /* see if we need to de-assert a reset pin */
> + chip->reset_gpio = devm_gpiod_get_optional(&client->dev,
> +"reset",
> +GPIOD_OUT_LOW);

> + if (IS_ERR(chip->reset_gpio)) {
> + dev_err(&client->dev, "request for reset pin failed\n");
> + return PTR_ERR(chip->reset_gpio);
> + }
> +
> + if (chip->reset_gpio) {
> + /* bring chip out of reset */
> + dev_info(&client->dev, "releasing reset\n");
> + gpiod_set_value(chip->reset_gpio, 0);
>
The pin is already initialized to the inactive state thru the
GPIOD_OUT_LOW flag in devm_gpiod_get_optional(), so this call to
gpiod_set_value() is useless.


Lothar Waßmann
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


[PATCH] imx-drm: parallel-display: honor 'native-mode' property when selecting video mode from DT

2014-01-13 Thread Lothar Waßmann
This patch allows to select a specific video mode from a list of modes
defined in DT by setting the 'native-mode' property appropriately.

Since all current users of this driver have only one mode defined in
their .dts files, the patch does not change the behaviour of this
driver on the affected platforms.

Signed-off-by: Lothar Waßmann 
---
 drivers/staging/imx-drm/parallel-display.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/staging/imx-drm/parallel-display.c 
b/drivers/staging/imx-drm/parallel-display.c
index 24aa9be..351d61d 100644
--- a/drivers/staging/imx-drm/parallel-display.c
+++ b/drivers/staging/imx-drm/parallel-display.c
@@ -23,6 +23,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "imx-drm.h"
 
@@ -74,7 +75,7 @@ static int imx_pd_connector_get_modes(struct drm_connector 
*connector)
 
if (np) {
struct drm_display_mode *mode = drm_mode_create(connector->dev);
-   of_get_drm_display_mode(np, &imxpd->mode, 0);
+   of_get_drm_display_mode(np, &imxpd->mode, OF_USE_NATIVE_MODE);
drm_mode_copy(mode, &imxpd->mode);
mode->type |= DRM_MODE_TYPE_DRIVER | DRM_MODE_TYPE_PREFERRED,
drm_mode_probed_add(connector, mode);
-- 
1.7.2.5

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv5] staging/iio/adc: change the MXS touchscreen driver implementation

2013-09-23 Thread Lothar Waßmann
Hi,

Jürgen Beisert writes:
> Hi Marek,
> 
> Seems the very first touch detection does not work. A colleague just told me, 
> we have an i.MX28 based hardware here and maybe a touchscreen from a 
> different 
> platform which maybe can be used with the i.MX28 platform. Stay tuned...
> 
> > btw. the touchscreen connection on M28EVK is pretty much the same as on
> > MX28EVK.
> 
> I think the touchscreen connection on MX28EVK is exactly the same as on 
> MX28EVK! ;) (/me needs more coffee as well)
> 
Just tested the touchscreen driver on our TX28 module and I could not
find any problems.


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv5] staging/iio/adc: change the MXS touchscreen driver implementation

2013-09-23 Thread Lothar Waßmann
Hi,

Jürgen Beisert writes:
> Hi Marek,
> 
> On Monday 23 September 2013 09:55:58 Jürgen Beisert wrote:
> > On Monday 23 September 2013 01:55:04 Marek Vasut wrote:
> > > > The following series replaces the current busy loop touchscreen
> > > > implementation for i.MX28/i.MX23 SoCs by a fully interrupt driven
> > > > implementation.
> > > >
> > > > Since i.MX23 and i.MX28 silicon differs, the existing implementation
> > > > can be used for the i.MX28 SoC only.
> > > >
> > > > So, the first two patches of this series move the i.MX28 specific
> > > > definitions out of the way. The third patch simplifies the register
> > > > access to make it easier to add the i.MX23 support. Then the i.MX23
> > > > specific definitions are added, also the code to distinguish both SoCs
> > > > at run-time. Up to here the existing touchscreen driver will now run on
> > > > an i.MX23 Soc as well.
> > > >
> > > > When the i.MX23 SoC is running from battery it seems not to be a good
> > > > idea to run a busy loop to detect touches and their location. The
> > > > fourth patch adds a fully interrupt driven implementation which makes
> > > > use of the built-in delay and multiple sample features of the
> > > > touchscreen controller. This will reduce the interrupt load to a
> > > > minimum.
> > > >
> > > > The next to last patch in this series just removes the existing busy
> > > > loop implementation.
> > > >
> > > > The last patch adds a devicetree bindings proposal (to be discussed).
> > > >
> > > > Some restrictions/questions:
> > > >
> > > > - the touchscreen part is yet tested on i.MX23 SoC only
> > > > - has someone a good idea how to implement a reliable pressure
> > > > measurement if the resistances of the touchscreen's plates are unknown?
> > > >
> > > > Changes since v4:
> > > >
> > > > - honor Jonathan's comments about function names
> > > > - honor Dmitry's comments about workqueue canceling and interrupts
> > > > - adding devicetree bindings proposal
> > > >
> > > > Changes since v3:
> > > >
> > > > - split adding register access functions and i.MX23 support into two
> > > > patches
> > > >
> > > > Changes since v2:
> > > >
> > > > - useless debug output removed
> > > >
> > > > Changes since v1:
> > > >
> > > > - adding register access functions to make the existing code more
> > > > readable - adding some functions to distinguish the SoCs at run-time to
> > > > avoid if-else contructs whenever differences in the register layout
> > > > between i.MX23 and i.MX28 must be handled
> > >
> > > Just tested this on M28EVK, the touchscreen is dead. I do not get any
> > > touch events when I use ts_calibrate and when I hd /dev/input/eventX , I
> > > get nothing either.
> > >
> > > Right now, I'm somehow on a tight schedule, but I'd like to see this
> > > resolved ASAP. Do you have any hint for me ?
> >
> > Seems the very first touch detection does not work. A colleague just told
> > me, we have an i.MX28 based hardware here and maybe a touchscreen from a
> > different platform which maybe can be used with the i.MX28 platform. Stay
> > tuned...
> 
> I also found an MX28EVK with a touchscreen here and: yes it does not work 
> with 
> the modified driver. But now I'm confused, because of Lothar's statement 
> about 
> the TX28 platform.
> 
> @Lothar: What kernel revision did you use? I had to go back to 3.9 to get a 
> 
Current linux-next (3.12-rc1).

> working kernel on the MX28EVK. More recent vanilla kernels stop working (MMC 
> and network fail here).
>
I'm seeing some messages:
|mxs-mmc 8001.ssp: dummy supplies not allowed
|fec 800f.ethernet: dummy supplies not allowed
But without any apparent consequences. Maybe that's a problem with the
MX28EVK?


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCHv5] staging/iio/adc: change the MXS touchscreen driver implementation

2013-09-23 Thread Lothar Waßmann
Hi,

Jürgen Beisert writes:
> The i.MX28 manual says the LRADC delay unit is driven by a 2 kHz clock, but 
> does not say what kind of clock it is (or is derived from). I guess now, this 
> clock isn't enabled and thus the delay unit can't work. At least on the 
> MX28EVK. On your TX28 platform the clock seems enabled and the delay unit can 
> do its job and drive the state machine.
>
The i.MX28 Ref Manual states in chapter 38.5.14 LRADC Scheduling Delay:
| This counter operates on a 2KHz clock derived from crystal clock.
Thus the clock should always be enabled.


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel


Re: [PATCH 1/9] Staging/iio/adc/touchscreen/MXS: add proper clock handling

2013-09-23 Thread Lothar Waßmann
Hi,

Juergen Beisert writes:
> diff --git a/drivers/staging/iio/adc/mxs-lradc.c 
> b/drivers/staging/iio/adc/mxs-lradc.c
> index a08c173..00b61ac 100644
> --- a/drivers/staging/iio/adc/mxs-lradc.c
> +++ b/drivers/staging/iio/adc/mxs-lradc.c
> @@ -35,6 +35,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include 
>  #include 
> @@ -134,6 +135,8 @@ struct mxs_lradc {
>   void __iomem*base;
>   int irq[13];
>  
> + struct clk  *clk;
> +
>   uint32_t*buffer;
>   struct iio_trigger  *trig;
>  
> @@ -928,6 +931,9 @@ static int mxs_lradc_probe(struct platform_device *pdev)
>   if (IS_ERR(lradc->base))
>   return PTR_ERR(lradc->base);
>  
> + lradc->clk = devm_clk_get(&pdev->dev, NULL);
> + clk_prepare_enable(lradc->clk);
> +
Wouldn't it make sense to enable the clock only when the device is
opened to save power while not actually in use?


Lothar Waßmann
-- 
___

Ka-Ro electronics GmbH | Pascalstraße 22 | D - 52076 Aachen
Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10
Geschäftsführer: Matthias Kaussen
Handelsregistereintrag: Amtsgericht Aachen, HRB 4996

www.karo-electronics.de | i...@karo-electronics.de
___
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel