[PATCH v5 2/2] drm/bridge: lvds-codec: Add support for pixel data sampling edge select

[Marek Vasut]

On 10/17/21 7:40 PM, Sam Ravnborg wrote:
> Hi Marek,
> 
> On Sun, Oct 17, 2021 at 07:29:51PM +0200, Marek Vasut wrote:
>> On 10/17/21 6:49 PM, Sam Ravnborg wrote:
>>
>> [...]
>>
>>>> +	/*
>>>> +	 * Encoder might sample data on different clock edge than the display,
>>>> +	 * for example OnSemi FIN3385 has a dedicated strapping pin to select
>>>> +	 * the sampling edge.
>>>> +	 */
>>>> +	if (lvds_codec->connector_type == DRM_MODE_CONNECTOR_LVDS &&
>>>> +	    !of_property_read_u32(dev->of_node, "pclk-sample", &val)) {
>>>> +		lvds_codec->timings.input_bus_flags = val ?
>>>> +			DRM_BUS_FLAG_PIXDATA_SAMPLE_POSEDGE :
>>>> +			DRM_BUS_FLAG_PIXDATA_SAMPLE_NEGEDGE;
>>>> +	}
>>>> +
>>>>    	/*
>>>>    	 * The panel_bridge bridge is attached to the panel's of_node,
>>>>    	 * but we need a bridge attached to our of_node for our user
>>>>    	 * to look up.
>>>>    	 */
>>>>    	lvds_codec->bridge.of_node = dev->of_node;
>>>> +	lvds_codec->bridge.timings = &lvds_codec->timings;
>>> I do not understand how this will work. The only field that is set is timings.input_bus_flags
>>> but any user will see bridge.timings is set and will think this is all
>>> timing info.
>>>
>>> Maybe I just misses something obvious?
>>
>> Is there anything else in those timings that should be set ? See
>> include/drm/drm_bridge.h around line 640
>>
>> setup_time_ps/hold_time_ps/dual_link isn't supported by this driver, so it
>> is 0 or false anyway, i.e. no change.
> 
> Just me being confused with display_timings. Patch looks good.
> Reviewed-by: Sam Ravnborg <sam at ravnborg.org>
> 
> Ping me in a few days to apply it if there is no more feedback.

ACK