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@xxxxxxxxxxxxxxxx>
>> >> ---
>> >>
>> >> 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
