[PATCH v2 04/13] drm: Add data mirror bus flag

[Stefan Agner]

On 2016-12-20 14:21, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thank you for the review.
> 
> On Tuesday 20 Dec 2016 14:01:46 Stefan Agner wrote:
>> On 2016-12-18 21:31, Laurent Pinchart wrote:
>> > Hi Stefan and Thierry,
>> >
>> > As the author and suggester of the other bus flags, could you please
>> > review this patch ?
>>
>> It looks to me like an appropriate use case for the flag. One remark
>> below:
>>
>> > On Saturday 19 Nov 2016 05:28:04 Laurent Pinchart wrote:
>> >> The flag indicates that data is mirrored on the bus. The exact meaning
>> >> is bus-type dependent. For LVDS buses it indicates that the seven data
>> >> bits that transmitted in a clock pulse are sent in slots 6 to 0 order.
>> >>
>> >> Signed-off-by: Laurent Pinchart
>> >> <laurent.pinchart+renesas at ideasonboard.com>
>> >> ---
>> >>
>> >>  include/drm/drm_connector.h | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
>> >> index ac9d7d8e0e43..5c1dda236760 100644
>> >> --- a/include/drm/drm_connector.h
>> >> +++ b/include/drm/drm_connector.h
>> >> @@ -159,6 +159,8 @@ struct drm_display_info {
>> >>
>> >>  #define DRM_BUS_FLAG_PIXDATA_POSEDGE	(1<<2)
>> >>  /* drive data on neg. edge */
>> >>  #define DRM_BUS_FLAG_PIXDATA_NEGEDGE	(1<<3)
>> >>
>> >> +/* data is mirrored on the bus */
>> >> +#define DRM_BUS_FLAG_DATA_MIRROR	(1<<4)
>>
>> Sounds like a bit endianness issue. I am wondering is if "mirror" is a
>> good term. Can we name the possible orderings? How about:
>>
>> DRM_BUS_FLAG_DATA_MSB_TO_LSB
>> DRM_BUS_FLAG_DATA_LSB_TO_MSB
> 
> LVDS display buses send pixels in RGB666 or RGB888 as follows.
> 
> - "jeida-18" - 18-bit data mapping compatible with the [JEIDA], [LDI] and
>   [VESA] specifications. Data are transferred as follows on 3 LVDS lanes.
> 
> Slot	    0       1       2       3       4       5       6
> 	________________                         _________________
> Clock	                \_______________________/
> 	  ______  ______  ______  ______  ______  ______  ______
> DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> 
> - "jeida-24" - 24-bit data mapping compatible with the [DSIM] and [LDI]
>   specifications. Data are transferred as follows on 4 LVDS lanes.
> 
> Slot	    0       1       2       3       4       5       6
> 	________________                         _________________
> Clock	                \_______________________/
> 	  ______  ______  ______  ______  ______  ______  ______
> DATA0	><__G2__><__R7__><__R6__><__R5__><__R4__><__R3__><__R2__><
> DATA1	><__B3__><__B2__><__G7__><__G6__><__G5__><__G4__><__G3__><
> DATA2	><_CTL2_><_CTL1_><_CTL0_><__B7__><__B6__><__B5__><__B4__><
> DATA3	><_CTL3_><__B1__><__B0__><__G1__><__G0__><__R1__><__R0__><
> 
> - "vesa-24" - 24-bit data mapping compatible with the [VESA] specification.
>   Data are transferred as follows on 4 LVDS lanes.
> 
> Slot	    0       1       2       3       4       5       6
> 	________________                         _________________
> Clock	                \_______________________/
> 	  ______  ______  ______  ______  ______  ______  ______
> DATA0	><__G0__><__R5__><__R4__><__R3__><__R2__><__R1__><__R0__><
> DATA1	><__B1__><__B0__><__G5__><__G4__><__G3__><__G2__><__G1__><
> DATA2	><_CTL2_><_CTL1_><_CTL0_><__B5__><__B4__><__B3__><__B2__><
> DATA3	><_CTL3_><__B7__><__B6__><__G7__><__G6__><__R7__><__R6__><
> 
> 
> Mirroring flips slots 0 to 6, resulting in a sort of LSB to MSB transmission 
> (I'm not sure I'd call that endianness though). I'm fine renaming the flag as 
> you propose. Do we need two flags, or should we assume MSB to LSB by default 
> and add a single flag ?

Wikipedia has a chapter about bit endianness, but I agree, I would not
call it that way; the term endianness usually refers to byte endianness.

For the other flags we usually have both variants. One variant is the
(unwritten) default for most displays (and most drivers/display
controllers configure it that way). And most displays only specify a
flag if they require the non-default setting.

I suggest to add both, just for the sake of completeness.

--
Stefan


> 
>> >>  	/**
>> >>  	 * @bus_flags: Additional information (like pixel signal polarity) for