diff mbox series

[v2,6/8] drm/imx: support handling bridge timings bus flags

Message ID 20180912183222.25414-7-stefan@agner.ch (mailing list archive)
State New, archived
Headers show
Series drm/bridge: add bus flag support | expand

Commit Message

Stefan Agner Sept. 12, 2018, 6:32 p.m. UTC
A bridge might require specific settings for the pixel data on
the bus. Copy the bus flags from the bridge timings if a bridge
is in use.

Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 drivers/gpu/drm/imx/parallel-display.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Philipp Zabel Sept. 13, 2018, 8:38 a.m. UTC | #1
On Wed, 2018-09-12 at 11:32 -0700, Stefan Agner wrote:
> A bridge might require specific settings for the pixel data on
> the bus.

On which bus? The bridge has input and output.

> Copy the bus flags from the bridge timings if a bridge
> is in use.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
> index aefd04e18f93..7798a0621df7 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
>  	if (ret && ret != -ENODEV)
>  		return ret;
>  
> +	if (imxpd->bridge && imxpd->bridge->timings)
> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;

Oh, ok. I'd also specify input bus in the commit message.

> +
>  	imxpd->dev = dev;
>  
>  	ret = imx_pd_register(drm, imxpd);

regards
Philipp
Stefan Agner Sept. 13, 2018, 5:03 p.m. UTC | #2
On 13.09.2018 01:38, Philipp Zabel wrote:
> On Wed, 2018-09-12 at 11:32 -0700, Stefan Agner wrote:
>> A bridge might require specific settings for the pixel data on
>> the bus.
> 
> On which bus? The bridge has input and output.
> 
>> Copy the bus flags from the bridge timings if a bridge
>> is in use.
>>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
>> index aefd04e18f93..7798a0621df7 100644
>> --- a/drivers/gpu/drm/imx/parallel-display.c
>> +++ b/drivers/gpu/drm/imx/parallel-display.c
>> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device *master, void *data)
>>  	if (ret && ret != -ENODEV)
>>  		return ret;
>>
>> +	if (imxpd->bridge && imxpd->bridge->timings)
>> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;
> 
> Oh, ok. I'd also specify input bus in the commit message.
> 

Good point, will change in v3 to:

"A bridge might require specific settings for the pixel data on
its input bus. Those are specified through optional bus timings.
Copy the bridges input bus flags as to the imxpd bus flags."

--
Stefan

>> +
>>  	imxpd->dev = dev;
>>
>>  	ret = imx_pd_register(drm, imxpd);
> 
> regards
> Philipp
Laurent Pinchart Sept. 14, 2018, 10 a.m. UTC | #3
Hi Stefan,

Thank you for the patch.

On Wednesday, 12 September 2018 21:32:20 EEST Stefan Agner wrote:
> A bridge might require specific settings for the pixel data on
> the bus. Copy the bus flags from the bridge timings if a bridge
> is in use.
> 
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  drivers/gpu/drm/imx/parallel-display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/imx/parallel-display.c
> b/drivers/gpu/drm/imx/parallel-display.c index aefd04e18f93..7798a0621df7
> 100644
> --- a/drivers/gpu/drm/imx/parallel-display.c
> +++ b/drivers/gpu/drm/imx/parallel-display.c
> @@ -239,6 +239,9 @@ static int imx_pd_bind(struct device *dev, struct device
> *master, void *data) if (ret && ret != -ENODEV)
>  		return ret;
> 
> +	if (imxpd->bridge && imxpd->bridge->timings)
> +		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;

As explained in different replies in this mail thread (and in v1), we need 
something more complex than this.

How about creating a helper function that would take a sampling edge, setup 
and hold times, clock frequency and internal delay on the driving side, and 
return which edge to drive the data on ? I agree with you that the logic is 
complex, so we shouldn't duplicate it in multiple drivers. If you submit such 
a patch I'll implement support for configuring the clock edge in the R-Car DU 
driver and test it with the ADV7123.

>  	imxpd->dev = dev;
> 
>  	ret = imx_pd_register(drm, imxpd);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/imx/parallel-display.c b/drivers/gpu/drm/imx/parallel-display.c
index aefd04e18f93..7798a0621df7 100644
--- a/drivers/gpu/drm/imx/parallel-display.c
+++ b/drivers/gpu/drm/imx/parallel-display.c
@@ -239,6 +239,9 @@  static int imx_pd_bind(struct device *dev, struct device *master, void *data)
 	if (ret && ret != -ENODEV)
 		return ret;
 
+	if (imxpd->bridge && imxpd->bridge->timings)
+		imxpd->bus_flags = imxpd->bridge->timings->input_bus_flags;
+
 	imxpd->dev = dev;
 
 	ret = imx_pd_register(drm, imxpd);