[PATCH v3 41/50] drm/bridge: ti-tfp410: Report input bus config through bridge timings

[Tomi Valkeinen]

On 15/03/2019 13:30, Peter Ujfalusi wrote:
> 
> 
> On 28/02/2019 12.31, Tomi Valkeinen wrote:
>> On 28/02/2019 12:27, Tomi Valkeinen wrote:
>>> Hi Laurent,
>>>
>>> On 11/02/2019 11:46, Laurent Pinchart wrote:
>>>
>>>> +	/* Get the sampling edge from the endpoint. */
>>>> +	of_property_read_u32(ep, "pclk-sample", &pclk_sample);
>>>> +	of_node_put(ep);
>>>> +
>>>> +	timings->input_bus_flags = DRM_BUS_FLAG_DE_HIGH;
>>>> +
>>>> +	switch (pclk_sample) {
>>>> +	case 0:
>>>> +		timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE
>>>> +					 |  DRM_BUS_FLAG_SYNC_SAMPLE_NEGEDGE;
>>>> +		break;
>>>> +	case 1:
>>>> +		timings->input_bus_flags |= DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE
>>>> +					 |  DRM_BUS_FLAG_SYNC_SAMPLE_POSEDGE;
>>>> +		break;
>>>> +	default:
>>>> +		return -EINVAL;
>>>> +	}
>>>
>>> The default for pclk_sample is just the opposite of what omapdrm's
>>> tfp410 used to do. The dts doc file also says that pclk-sample is
>>> required, but the driver works fine without it, defaulting to 0.
>>>
>>> This means that none of the omap dts files with tfp410 work correctly,
>>> instead they silently use the wrong settings which may work but easily
>>> also won't...
>>>
>>> As the bus flags are added in this patch for the first time, maybe we
>>> can assume that no one is using them, and the default could be made to
>>> be the same as was on omapdrm's tfp410?
>>
>> Aaaand never mind. In omapdrm's driver we were using
>> DRM_BUS_FLAG_SYNC_DRIVE_* variant, here we have SAMPLE variant. So it's
>> fine =).
> 
> If the pclk-sample is not defined in DT, it will default to 0 which
> selects SAMPLE_NEGEDGE (== DRIVE_POSEDGE), right?
> 
> But all the boards where I can find schematics with tfp410 have their
> EDGE/HTPLG pin pulled up and according to the documentation when EDGE=1
> then tfp410 will sample on the rising edge.
> 
> imho the pclk_sample should be initialized to 1 to avoid regression for
> most of the boards using tfp410.

Define "regression" =). If the omapdrm driver was always using
DRIVE_POSEDGE, this driver should also be using DRIVE_POSEDGE, no? If it
does the same as the old driver, it can't be a regression. This is, of
course, only considering omapdrm based boards.

That said, it sounds odd that this would be wrong in the old driver, but
then again, it might well be, as code related to these sync signals has
changed sooo many times, and the related DSS registers are somewhat
confusing.

That gave me an idea, I need to write an rwmem script to print the DSS
sync flags in plain english...

 Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki