From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Subject: Re: [PATCH v17 2/7] video: add display_timing and videomode Date: Wed, 27 Feb 2013 18:13:49 +0200 Message-ID: <512E30BD.7010603@ti.com> References: <1359104515-8907-1-git-send-email-s.trumtrar@pengutronix.de> <1359104515-8907-3-git-send-email-s.trumtrar@pengutronix.de> <51223615.4090709@iki.fi> <512E2A1B.6040704@ti.com> <20130227160540.GA10491@pengutronix.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8946138499221894002==" Return-path: In-Reply-To: <20130227160540.GA10491-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: devicetree-discuss-bounces+gldd-devicetree-discuss=m.gmane.org-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org Sender: "devicetree-discuss" To: Steffen Trumtrar Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "Mohammed, Afzal" , Dave Airlie , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, Florian Tobias Schandinat , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Rob Clark , Laurent Pinchart , kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, Guennady Liakhovetski , linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: devicetree@vger.kernel.org --===============8946138499221894002== Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig6E8BABB66675C1BD3801463B" --------------enig6E8BABB66675C1BD3801463B Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2013-02-27 18:05, Steffen Trumtrar wrote: > Ah, sorry. Forgot to answer this. >=20 > On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote: >> Ping. >> >> On 2013-02-18 16:09, Tomi Valkeinen wrote: >>> Hi Steffen, >>> >>> On 2013-01-25 11:01, Steffen Trumtrar wrote: >>> >>>> +/* VESA display monitor timing parameters */ >>>> +#define VESA_DMT_HSYNC_LOW BIT(0) >>>> +#define VESA_DMT_HSYNC_HIGH BIT(1) >>>> +#define VESA_DMT_VSYNC_LOW BIT(2) >>>> +#define VESA_DMT_VSYNC_HIGH BIT(3) >>>> + >>>> +/* display specific flags */ >>>> +#define DISPLAY_FLAGS_DE_LOW BIT(0) /* data enable flag */ >>>> +#define DISPLAY_FLAGS_DE_HIGH BIT(1) >>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE BIT(2) /* drive data on pos. = edge */ >>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE BIT(3) /* drive data on neg. = edge */ >>>> +#define DISPLAY_FLAGS_INTERLACED BIT(4) >>>> +#define DISPLAY_FLAGS_DOUBLESCAN BIT(5) >>> >>> >>> >>>> + unsigned int dmt_flags; /* VESA DMT flags */ >>>> + unsigned int data_flags; /* video data flags */ >>> >>> Why did you go for this approach? To be able to represent >>> true/false/not-specified? >>> >=20 > We decided somewhere between v3 and v8 (I think), that those flags can = be > high/low/ignored. Okay. Why aren't they enums, though? That always makes more clear which defines are to be used with which fields. >>> Would it be simpler to just have "flags" field? What does it give us = to >>> have those two separately? >>> >=20 > I decided to split them, so it is clear that some flags are VESA define= d and > the others are "invented" for the display-timings framework and may be > extended. Hmm... Okay. Is it relevant that they are VESA defined? It just feels to complicate handling the flags =3D). >>> Should the above say raising edge/falling edge instead of positive >>> edge/negative edge? >>> >=20 > Hm, I used posedge/negedge because it is shorter (and because of my Ver= ilog past > pretty natural to me :-) ). I don't know what others are thinking thoug= h. I guess it's quite clear, but it's still different terms than used elsewhere, e.g. documentation for videomodes. Another thing I noticed while using the new videomode, display_timings.h has a few names that are quite short and generic. Like "TE_MIN", which is now a global define. And "timing_entry". Either name could be well used internally in some .c file, and could easily clash. Tomi --------------enig6E8BABB66675C1BD3801463B Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBAgAGBQJRLjC9AAoJEPo9qoy8lh71m+0P/RPDx491Eic5r4Yc0NlEDMf0 VL64T3j2dns71crKbMkctgiPOBZDjCzp08xd4UsqNj7hkgu94p+5OyHfmZXq7xJw ycJElBI+1y3IhnRWOq+vUgUlGgUFPfDY1OeyKflk8dTK/HwcQex3yyt//kLJS4Ge 971IZrkVPFnUi00LTgxOf2wVIq5bms+sEG0hbGes7qFkejfkYDMNIUNVt2nRXpf7 aC8SpmPOsOp5MDeTKCP17m0N8/A7rGM03O9PV6L+xvq6sDRRaBHwLhQG3sCMnvP+ 3OCTmB9EDJOYxmLIu9aqCT0RccZl+y61HYZJm7aXO8iZ+G4NK/62wbO5UDJB7hvl 6/KIDWM1kz33jynXVOB8aLViyabQ9bqaWGo5waKJKelsZb8G7JrHpfJC/JiN6krv LofFbl+C8UEI3kRoDhJ4MZX2OkFy+HeAMQHSKkLUwAYoA9OamgIDK5Em6P1W0X6V N1gfDxXbXW5wJ+YBDhs6eauZzVvPNWmWZbB4vL+5JX0et+QtBypEwX56lVUZ7bOD cQA762XeYHGkiruzx7idxuw29w9lK7J1xHVqnu6PfLf5Lndgv+lFzphepz1v6mCx zJUiDNp/3l8U86eOfoRmriFr2l90i7hkOWQLy1cibBy/gip/KiDtWdWJExvkRkmA i1TyIWKhCK+12h3e7//4 =Lf4m -----END PGP SIGNATURE----- --------------enig6E8BABB66675C1BD3801463B-- --===============8946138499221894002== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ devicetree-discuss mailing list devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org https://lists.ozlabs.org/listinfo/devicetree-discuss --===============8946138499221894002==-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from arroyo.ext.ti.com ([192.94.94.40]:38728 "EHLO arroyo.ext.ti.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1760049Ab3B0QOP (ORCPT ); Wed, 27 Feb 2013 11:14:15 -0500 Message-ID: <512E30BD.7010603@ti.com> Date: Wed, 27 Feb 2013 18:13:49 +0200 From: Tomi Valkeinen MIME-Version: 1.0 To: Steffen Trumtrar CC: , Dave Airlie , Rob Herring , , , Laurent Pinchart , Thierry Reding , Guennady Liakhovetski , , Stephen Warren , Florian Tobias Schandinat , Rob Clark , Leela Krishna Amudala , "Mohammed, Afzal" , Subject: Re: [PATCH v17 2/7] video: add display_timing and videomode References: <1359104515-8907-1-git-send-email-s.trumtrar@pengutronix.de> <1359104515-8907-3-git-send-email-s.trumtrar@pengutronix.de> <51223615.4090709@iki.fi> <512E2A1B.6040704@ti.com> <20130227160540.GA10491@pengutronix.de> In-Reply-To: <20130227160540.GA10491@pengutronix.de> Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="------------enig6E8BABB66675C1BD3801463B" Sender: linux-media-owner@vger.kernel.org List-ID: --------------enig6E8BABB66675C1BD3801463B Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2013-02-27 18:05, Steffen Trumtrar wrote: > Ah, sorry. Forgot to answer this. >=20 > On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote: >> Ping. >> >> On 2013-02-18 16:09, Tomi Valkeinen wrote: >>> Hi Steffen, >>> >>> On 2013-01-25 11:01, Steffen Trumtrar wrote: >>> >>>> +/* VESA display monitor timing parameters */ >>>> +#define VESA_DMT_HSYNC_LOW BIT(0) >>>> +#define VESA_DMT_HSYNC_HIGH BIT(1) >>>> +#define VESA_DMT_VSYNC_LOW BIT(2) >>>> +#define VESA_DMT_VSYNC_HIGH BIT(3) >>>> + >>>> +/* display specific flags */ >>>> +#define DISPLAY_FLAGS_DE_LOW BIT(0) /* data enable flag */ >>>> +#define DISPLAY_FLAGS_DE_HIGH BIT(1) >>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE BIT(2) /* drive data on pos. = edge */ >>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE BIT(3) /* drive data on neg. = edge */ >>>> +#define DISPLAY_FLAGS_INTERLACED BIT(4) >>>> +#define DISPLAY_FLAGS_DOUBLESCAN BIT(5) >>> >>> >>> >>>> + unsigned int dmt_flags; /* VESA DMT flags */ >>>> + unsigned int data_flags; /* video data flags */ >>> >>> Why did you go for this approach? To be able to represent >>> true/false/not-specified? >>> >=20 > We decided somewhere between v3 and v8 (I think), that those flags can = be > high/low/ignored. Okay. Why aren't they enums, though? That always makes more clear which defines are to be used with which fields. >>> Would it be simpler to just have "flags" field? What does it give us = to >>> have those two separately? >>> >=20 > I decided to split them, so it is clear that some flags are VESA define= d and > the others are "invented" for the display-timings framework and may be > extended. Hmm... Okay. Is it relevant that they are VESA defined? It just feels to complicate handling the flags =3D). >>> Should the above say raising edge/falling edge instead of positive >>> edge/negative edge? >>> >=20 > Hm, I used posedge/negedge because it is shorter (and because of my Ver= ilog past > pretty natural to me :-) ). I don't know what others are thinking thoug= h. I guess it's quite clear, but it's still different terms than used elsewhere, e.g. documentation for videomodes. Another thing I noticed while using the new videomode, display_timings.h has a few names that are quite short and generic. Like "TE_MIN", which is now a global define. And "timing_entry". Either name could be well used internally in some .c file, and could easily clash. Tomi --------------enig6E8BABB66675C1BD3801463B Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBAgAGBQJRLjC9AAoJEPo9qoy8lh71m+0P/RPDx491Eic5r4Yc0NlEDMf0 VL64T3j2dns71crKbMkctgiPOBZDjCzp08xd4UsqNj7hkgu94p+5OyHfmZXq7xJw ycJElBI+1y3IhnRWOq+vUgUlGgUFPfDY1OeyKflk8dTK/HwcQex3yyt//kLJS4Ge 971IZrkVPFnUi00LTgxOf2wVIq5bms+sEG0hbGes7qFkejfkYDMNIUNVt2nRXpf7 aC8SpmPOsOp5MDeTKCP17m0N8/A7rGM03O9PV6L+xvq6sDRRaBHwLhQG3sCMnvP+ 3OCTmB9EDJOYxmLIu9aqCT0RccZl+y61HYZJm7aXO8iZ+G4NK/62wbO5UDJB7hvl 6/KIDWM1kz33jynXVOB8aLViyabQ9bqaWGo5waKJKelsZb8G7JrHpfJC/JiN6krv LofFbl+C8UEI3kRoDhJ4MZX2OkFy+HeAMQHSKkLUwAYoA9OamgIDK5Em6P1W0X6V N1gfDxXbXW5wJ+YBDhs6eauZzVvPNWmWZbB4vL+5JX0et+QtBypEwX56lVUZ7bOD cQA762XeYHGkiruzx7idxuw29w9lK7J1xHVqnu6PfLf5Lndgv+lFzphepz1v6mCx zJUiDNp/3l8U86eOfoRmriFr2l90i7hkOWQLy1cibBy/gip/KiDtWdWJExvkRkmA i1TyIWKhCK+12h3e7//4 =Lf4m -----END PGP SIGNATURE----- --------------enig6E8BABB66675C1BD3801463B-- From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tomi Valkeinen Date: Wed, 27 Feb 2013 16:13:49 +0000 Subject: Re: [PATCH v17 2/7] video: add display_timing and videomode Message-Id: <512E30BD.7010603@ti.com> MIME-Version: 1 Content-Type: multipart/mixed; boundary="------------enig6E8BABB66675C1BD3801463B" List-Id: References: <1359104515-8907-1-git-send-email-s.trumtrar@pengutronix.de> <1359104515-8907-3-git-send-email-s.trumtrar@pengutronix.de> <51223615.4090709@iki.fi> <512E2A1B.6040704@ti.com> <20130227160540.GA10491@pengutronix.de> In-Reply-To: <20130227160540.GA10491-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org> To: Steffen Trumtrar Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, "Mohammed, Afzal" , Dave Airlie , devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org, Florian Tobias Schandinat , dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org, Rob Clark , Laurent Pinchart , kernel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org, Guennady Liakhovetski , linux-media-u79uwXL29TY76Z2rM5mHXA@public.gmane.org --------------enig6E8BABB66675C1BD3801463B Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On 2013-02-27 18:05, Steffen Trumtrar wrote: > Ah, sorry. Forgot to answer this. >=20 > On Wed, Feb 27, 2013 at 05:45:31PM +0200, Tomi Valkeinen wrote: >> Ping. >> >> On 2013-02-18 16:09, Tomi Valkeinen wrote: >>> Hi Steffen, >>> >>> On 2013-01-25 11:01, Steffen Trumtrar wrote: >>> >>>> +/* VESA display monitor timing parameters */ >>>> +#define VESA_DMT_HSYNC_LOW BIT(0) >>>> +#define VESA_DMT_HSYNC_HIGH BIT(1) >>>> +#define VESA_DMT_VSYNC_LOW BIT(2) >>>> +#define VESA_DMT_VSYNC_HIGH BIT(3) >>>> + >>>> +/* display specific flags */ >>>> +#define DISPLAY_FLAGS_DE_LOW BIT(0) /* data enable flag */ >>>> +#define DISPLAY_FLAGS_DE_HIGH BIT(1) >>>> +#define DISPLAY_FLAGS_PIXDATA_POSEDGE BIT(2) /* drive data on pos. = edge */ >>>> +#define DISPLAY_FLAGS_PIXDATA_NEGEDGE BIT(3) /* drive data on neg. = edge */ >>>> +#define DISPLAY_FLAGS_INTERLACED BIT(4) >>>> +#define DISPLAY_FLAGS_DOUBLESCAN BIT(5) >>> >>> >>> >>>> + unsigned int dmt_flags; /* VESA DMT flags */ >>>> + unsigned int data_flags; /* video data flags */ >>> >>> Why did you go for this approach? To be able to represent >>> true/false/not-specified? >>> >=20 > We decided somewhere between v3 and v8 (I think), that those flags can = be > high/low/ignored. Okay. Why aren't they enums, though? That always makes more clear which defines are to be used with which fields. >>> Would it be simpler to just have "flags" field? What does it give us = to >>> have those two separately? >>> >=20 > I decided to split them, so it is clear that some flags are VESA define= d and > the others are "invented" for the display-timings framework and may be > extended. Hmm... Okay. Is it relevant that they are VESA defined? It just feels to complicate handling the flags =3D). >>> Should the above say raising edge/falling edge instead of positive >>> edge/negative edge? >>> >=20 > Hm, I used posedge/negedge because it is shorter (and because of my Ver= ilog past > pretty natural to me :-) ). I don't know what others are thinking thoug= h. I guess it's quite clear, but it's still different terms than used elsewhere, e.g. documentation for videomodes. Another thing I noticed while using the new videomode, display_timings.h has a few names that are quite short and generic. Like "TE_MIN", which is now a global define. And "timing_entry". Either name could be well used internally in some .c file, and could easily clash. Tomi --------------enig6E8BABB66675C1BD3801463B Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (GNU/Linux) Comment: Using GnuPG with undefined - http://www.enigmail.net/ iQIcBAEBAgAGBQJRLjC9AAoJEPo9qoy8lh71m+0P/RPDx491Eic5r4Yc0NlEDMf0 VL64T3j2dns71crKbMkctgiPOBZDjCzp08xd4UsqNj7hkgu94p+5OyHfmZXq7xJw ycJElBI+1y3IhnRWOq+vUgUlGgUFPfDY1OeyKflk8dTK/HwcQex3yyt//kLJS4Ge 971IZrkVPFnUi00LTgxOf2wVIq5bms+sEG0hbGes7qFkejfkYDMNIUNVt2nRXpf7 aC8SpmPOsOp5MDeTKCP17m0N8/A7rGM03O9PV6L+xvq6sDRRaBHwLhQG3sCMnvP+ 3OCTmB9EDJOYxmLIu9aqCT0RccZl+y61HYZJm7aXO8iZ+G4NK/62wbO5UDJB7hvl 6/KIDWM1kz33jynXVOB8aLViyabQ9bqaWGo5waKJKelsZb8G7JrHpfJC/JiN6krv LofFbl+C8UEI3kRoDhJ4MZX2OkFy+HeAMQHSKkLUwAYoA9OamgIDK5Em6P1W0X6V N1gfDxXbXW5wJ+YBDhs6eauZzVvPNWmWZbB4vL+5JX0et+QtBypEwX56lVUZ7bOD cQA762XeYHGkiruzx7idxuw29w9lK7J1xHVqnu6PfLf5Lndgv+lFzphepz1v6mCx zJUiDNp/3l8U86eOfoRmriFr2l90i7hkOWQLy1cibBy/gip/KiDtWdWJExvkRkmA i1TyIWKhCK+12h3e7//4 =Lf4m -----END PGP SIGNATURE----- --------------enig6E8BABB66675C1BD3801463B--