Re: [PATCH v14 04/10] imx-drm: use defines for clock polarity settings
On Wed, Jun 25, 2014 at 11:44:47AM +0200, Denis Carikli wrote: On 06/25/2014 06:48 AM, Sascha Hauer wrote: +#define ENABLE_POL_LOW 0 +#define ENABLE_POL_HIGH1 Adding defines without a proper namespace (IPU_) outside a driver private header file is not nice. Anyway, instead of adding the defines ... Fixed in imx-drm: use defines for clock polarity settings and in imx-drm: Use drm_display_mode timings flags.. Denis, can you send just this one updated patch, so I can update the one I have here with this change. Once you've done that, I'll send the first four off to Greg. Thanks. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v14 04/10] imx-drm: use defines for clock polarity settings
On Wed, Jun 25, 2014 at 06:48:45AM +0200, Sascha Hauer wrote:
On Mon, Jun 16, 2014 at 12:11:18PM +0200, Denis Carikli wrote:
+
/*
* Bitfield of Display Interface signal polarities.
*/
@@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
unsigned clksel_en:1;
unsigned clkidle_en:1;
unsigned data_pol:1;/* true = inverted */
- unsigned clk_pol:1; /* true = rising edge */
+ unsigned clk_pol:1;
unsigned enable_pol:1;
unsigned Hsync_pol:1; /* true = active high */
unsigned Vsync_pol:1;
...can we rename the flags to more meaningful names instead?
unsigned clk_pol_rising_edge:1;
unsigned enable_pol_high:1;
unsigned hsync_active_high:1;
unsigned vsync_active_high:1;
Now look at patch 7, where these become tri-state:
- don't change
- rising edge/active high
- falling edge/active low
So your suggestion is not a good idea.
--
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v14 04/10] imx-drm: use defines for clock polarity settings
On 06/25/2014 06:48 AM, Sascha Hauer wrote: +#define ENABLE_POL_LOW 0 +#define ENABLE_POL_HIGH1 Adding defines without a proper namespace (IPU_) outside a driver private header file is not nice. Anyway, instead of adding the defines ... Fixed in imx-drm: use defines for clock polarity settings and in imx-drm: Use drm_display_mode timings flags.. Denis. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v14 04/10] imx-drm: use defines for clock polarity settings
On Wed, Jun 25, 2014 at 09:43:27AM +0100, Russell King - ARM Linux wrote:
On Wed, Jun 25, 2014 at 06:48:45AM +0200, Sascha Hauer wrote:
On Mon, Jun 16, 2014 at 12:11:18PM +0200, Denis Carikli wrote:
+
/*
* Bitfield of Display Interface signal polarities.
*/
@@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
unsigned clksel_en:1;
unsigned clkidle_en:1;
unsigned data_pol:1;/* true = inverted */
- unsigned clk_pol:1; /* true = rising edge */
+ unsigned clk_pol:1;
unsigned enable_pol:1;
unsigned Hsync_pol:1; /* true = active high */
unsigned Vsync_pol:1;
...can we rename the flags to more meaningful names instead?
unsigned clk_pol_rising_edge:1;
unsigned enable_pol_high:1;
unsigned hsync_active_high:1;
unsigned vsync_active_high:1;
Now look at patch 7, where these become tri-state:
- don't change
- rising edge/active high
- falling edge/active low
So your suggestion is not a good idea.
Hm, you're right.
Still I think we should add a prefix to make the context of the flags
clear.
Sascha
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917- |
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v14 04/10] imx-drm: use defines for clock polarity settings
On Mon, Jun 16, 2014 at 12:11:18PM +0200, Denis Carikli wrote: Signed-off-by: Denis Carikli de...@eukrea.com It would be nice to have a little more explanation in the commit messages for these patches. If you'd like to send me better commit messages for these patches, I'll add them to what I already have: imx-drm: use defines for clock polarity settings imx-drm: add RGB666 support for parallel display. It may also be worth describing the RGB666 format in the commit message for: v4l2: add new V4L2_PIX_FMT_RGB666 pixel format. And... getting some more acks for these patches would be very useful, I think I'd like to see Sascha's ack for these... Sascha? -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v14 04/10] imx-drm: use defines for clock polarity settings
On 06/24/2014 05:13 PM, Russell King - ARM Linux wrote: [...] If you'd like to send me better commit messages for these patches, I'll add them to what I already have: imx-drm: use defines for clock polarity settings The comment of the clk_pol field of the ipu_di_signal_cfg struct was inverted. Instead of merely inverting the comment, the values of clk_pol were defined. imx-drm: add RGB666 support for parallel display. This permits to drive parallel displays that expect the RGB666 color format. It may also be worth describing the RGB666 format in the commit message for: v4l2: add new V4L2_PIX_FMT_RGB666 pixel format. The RGB666 color format encodes 6 bits for each color(red, green and blue), linearly. It looks like this in memory: 017 RRGGBB Denis. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v14 04/10] imx-drm: use defines for clock polarity settings
On Tue, Jun 24, 2014 at 06:25:19PM +0200, Denis Carikli wrote: On 06/24/2014 05:13 PM, Russell King - ARM Linux wrote: [...] If you'd like to send me better commit messages for these patches, I'll add them to what I already have: imx-drm: use defines for clock polarity settings The comment of the clk_pol field of the ipu_di_signal_cfg struct was inverted. Instead of merely inverting the comment, the values of clk_pol were defined. s/inverting/fixing/ imx-drm: add RGB666 support for parallel display. This permits to drive parallel displays that expect the RGB666 color format. This allows imx-drm to drive ... It may also be worth describing the RGB666 format in the commit message for: v4l2: add new V4L2_PIX_FMT_RGB666 pixel format. The RGB666 color format encodes 6 bits for each color(red, green and blue), linearly. It looks like this in memory: 017 RRGGBB Thanks! I've tweaked them very slightly as detailed above so they read a bit better. -- FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly improving, and getting towards what was expected from it. ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v14 04/10] imx-drm: use defines for clock polarity settings
Signed-off-by: Denis Carikli de...@eukrea.com
---
ChangeLog v13-v14:
- Rebased
ChangeLog 12-v13:
- No changes
ChangeLog 11-v12:
- Improved the define names to match the hardware:
ENABLE_POL is not a clock signal but instead an enable signal.
ChangeLog v9-v10:
- New patch which was splitted out from:
staging: imx-drm: Use de-active and pixelclk-active display-timings..
- Fixes many issues in staging: imx-drm: Use de-active and pixelclk-active
display-timings.:
- More clear meaning of the polarity settings.
- The SET_CLK_POL and SET_DE_POL masks are not
needed anymore.
---
drivers/gpu/ipu-v3/ipu-di.c |4 ++--
drivers/staging/imx-drm/ipuv3-crtc.c |4 ++--
include/video/imx-ipu-v3.h |8 +++-
3 files changed, 11 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-di.c b/drivers/gpu/ipu-v3/ipu-di.c
index c490ba4..d00f357 100644
--- a/drivers/gpu/ipu-v3/ipu-di.c
+++ b/drivers/gpu/ipu-v3/ipu-di.c
@@ -595,7 +595,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct
ipu_di_signal_cfg *sig)
}
}
- if (sig-clk_pol)
+ if (sig-clk_pol == CLK_POL_POSEDGE)
di_gen |= DI_GEN_POLARITY_DISP_CLK;
ipu_di_write(di, di_gen, DI_GENERAL);
@@ -606,7 +606,7 @@ int ipu_di_init_sync_panel(struct ipu_di *di, struct
ipu_di_signal_cfg *sig)
reg = ipu_di_read(di, DI_POL);
reg = ~(DI_POL_DRDY_DATA_POLARITY | DI_POL_DRDY_POLARITY_15);
- if (sig-enable_pol)
+ if (sig-enable_pol == ENABLE_POL_HIGH)
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 720868b..7fec438 100644
--- a/drivers/staging/imx-drm/ipuv3-crtc.c
+++ b/drivers/staging/imx-drm/ipuv3-crtc.c
@@ -165,8 +165,8 @@ 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 = 1;
- sig_cfg.clk_pol = 0;
+ sig_cfg.enable_pol = ENABLE_POL_HIGH;
+ sig_cfg.clk_pol = CLK_POL_NEGEDGE;
sig_cfg.width = mode-hdisplay;
sig_cfg.height = mode-vdisplay;
sig_cfg.pixel_fmt = out_pixel_fmt;
diff --git a/include/video/imx-ipu-v3.h b/include/video/imx-ipu-v3.h
index 3e43e22..305 100644
--- a/include/video/imx-ipu-v3.h
+++ b/include/video/imx-ipu-v3.h
@@ -27,6 +27,12 @@ enum ipuv3_type {
#define IPU_PIX_FMT_GBR24 v4l2_fourcc('G', 'B', 'R', '3')
+#define CLK_POL_NEGEDGE0
+#define CLK_POL_POSEDGE1
+
+#define ENABLE_POL_LOW 0
+#define ENABLE_POL_HIGH1
+
/*
* Bitfield of Display Interface signal polarities.
*/
@@ -37,7 +43,7 @@ struct ipu_di_signal_cfg {
unsigned clksel_en:1;
unsigned clkidle_en:1;
unsigned data_pol:1;/* true = inverted */
- unsigned clk_pol:1; /* true = rising edge */
+ unsigned clk_pol:1;
unsigned enable_pol:1;
unsigned Hsync_pol:1; /* true = active high */
unsigned Vsync_pol:1;
--
1.7.9.5
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
