Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Dr. H. Nikolaus Schaller @ 2013-10-10 13:42 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Tomi Valkeinen, Marek Belisko, plagnioj, linux-kernel, linux-omap,
	linux-fbdev
In-Reply-To: <52569CE8.5070201@metafoo.de>


Am 10.10.2013 um 14:26 schrieb Lars-Peter Clausen:

> On 10/10/2013 02:13 PM, Tomi Valkeinen wrote:
>> On 10/10/13 14:52, Dr. H. Nikolaus Schaller wrote:
>> 
>>> Yes, I agree and I am willing to help if someone comes up with such a SoC.
>>> At the moment we have connected it to the OMAP3 only.
>> 
>> True, but even without that kind of SoC, SPI bitbanging should be
>> handled by an SPI driver, not by the drivers that use the bus.
>> 
>>> I.e. I want not to do a lot of work for others who we just guess about that they
>>> might exist...
>> 
>> Yep. It's fine for me, it's not that much extra code in the panel driver.
>> 
>>>> The panel hardware has three wires, so the panel driver (if it does
>>>> handle the bus by bitbanging) can only refer to three gpios.
>>> 
>>> Hm. The panle hardware has 3 but the SoC (OMAP3) the driver
>>> is running on has 4.
>> 
>> Right, but this panel driver is a driver for the panel hardware. Not for
>> the SoC, or the SoC+panel combination. So the panel driver must only use
>> resources as seen by the panel hardware.
>> 
>>>> So either
>>>> the bus details should be hidden by using the normal spi framework, or
>>>> if the driver does the bitbanging, use the gpios as specified in the
>>>> panel spec. The panel driver cannot contain any board specific stuff.
>>> 
>>> The bitbang driver shown below can handle either 3 or 4 gpios (except
>>> for initialization).
>> 
>> It's not a bitbang driver, it's a panel driver. And anyway, if I
>> understood right, your use of 4 gpios was just a hack to try to make it
>> look like a normal 4-wire SPI bus. What you really have is 3 wires, 3
>> gpios. I don't see any reason to use 4 gpios, as two of them are the same.

There is a protection resistor in the one defined as "output" so that
protocol errors don't have two outputs work against each other.

>> 
>> Hmm, how does it work anyway. Did I understand it right, the panel's
>> 'DIN' pin is connected to two gpios on the SoC, and one of those gpios
>> is set as output, and the other as input?

Yes.

>> So the SoC is always pulling
>> that line up or down, and the panel is also pulling it up or down when
>> it's sending data. I'm no HW guy but that sounds quite bad =).

Yes, that is the strange thing with this protocol. It does a direction reversal
after some time. I.e. the panel switches its input to output and the SoC has
to be fast enough to do the same. Therefore, we have one output GPIO
(protected by a resistor) and a separate input GPIO.

Sometimes interfacing hardware is indeed strange.

>> 
>> I've never written or studied a bitbanging driver, but shouldn't there
>> be just one gpio used on the SoC for DIN, and it would be set to either
>> output or input mode, depending on if we are reading or writing?
> 
> Back in the OpenMoko days we used the panel in normal 4-wire SPI mode with
> the GPIO bitbang SPI master. The bitbang code in this driver also looks like
> normal 4 wire.


Thanks for this hint!

Maybe using a standard 4-wire SPI simply works if we only write data and
make sure the panel is not sending anything...

I still hesitate to break working code because it needs quite some time to debug
and we don't even know if it ever works, i.e. sends us to swampy ground...

BR,
Nikolaus



^ permalink raw reply

* linux-next: manual merge of the omap_dss2 tree
From: Mark Brown @ 2013-10-10 17:50 UTC (permalink / raw)
  To: Archit Taneja, Tomi Valkeinen, Ricardo Neri
  Cc: linux-omap, linux-fbdev, linux-next, linux-kernel, Thierry Reding

[-- Attachment #1: Type: text/plain, Size: 346 bytes --]

Today's linux-next merge of the omap_dss2 tree got conflicts in
drivers/video/omap2/dss/hdmi4_core.[ch] caused by ef26958a (omapdss:
HDMI: Rename hdmi driver files to nicer names) interacting with a range
of commits from Ricardo Neri in the fbdev tree and possibly some other
stuff.  git is unfortunately not giving me a useful diff right now :/

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Lars-Peter Clausen @ 2013-10-10 18:58 UTC (permalink / raw)
  To: Dr. H. Nikolaus Schaller
  Cc: Tomi Valkeinen, Marek Belisko, plagnioj, linux-kernel, linux-omap,
	linux-fbdev
In-Reply-To: <A7739B52-2632-4DD8-924D-B657A6BFE3ED@goldelico.com>

On 10/10/2013 03:42 PM, Dr. H. Nikolaus Schaller wrote:
>
> Am 10.10.2013 um 14:26 schrieb Lars-Peter Clausen:
>
>> On 10/10/2013 02:13 PM, Tomi Valkeinen wrote:
>>> On 10/10/13 14:52, Dr. H. Nikolaus Schaller wrote:
>>>
>>>> Yes, I agree and I am willing to help if someone comes up with such a SoC.
>>>> At the moment we have connected it to the OMAP3 only.
>>>
>>> True, but even without that kind of SoC, SPI bitbanging should be
>>> handled by an SPI driver, not by the drivers that use the bus.
>>>
>>>> I.e. I want not to do a lot of work for others who we just guess about that they
>>>> might exist...
>>>
>>> Yep. It's fine for me, it's not that much extra code in the panel driver.
>>>
>>>>> The panel hardware has three wires, so the panel driver (if it does
>>>>> handle the bus by bitbanging) can only refer to three gpios.
>>>>
>>>> Hm. The panle hardware has 3 but the SoC (OMAP3) the driver
>>>> is running on has 4.
>>>
>>> Right, but this panel driver is a driver for the panel hardware. Not for
>>> the SoC, or the SoC+panel combination. So the panel driver must only use
>>> resources as seen by the panel hardware.
>>>
>>>>> So either
>>>>> the bus details should be hidden by using the normal spi framework, or
>>>>> if the driver does the bitbanging, use the gpios as specified in the
>>>>> panel spec. The panel driver cannot contain any board specific stuff.
>>>>
>>>> The bitbang driver shown below can handle either 3 or 4 gpios (except
>>>> for initialization).
>>>
>>> It's not a bitbang driver, it's a panel driver. And anyway, if I
>>> understood right, your use of 4 gpios was just a hack to try to make it
>>> look like a normal 4-wire SPI bus. What you really have is 3 wires, 3
>>> gpios. I don't see any reason to use 4 gpios, as two of them are the same.
>
> There is a protection resistor in the one defined as "output" so that
> protocol errors don't have two outputs work against each other.
>
>>>
>>> Hmm, how does it work anyway. Did I understand it right, the panel's
>>> 'DIN' pin is connected to two gpios on the SoC, and one of those gpios
>>> is set as output, and the other as input?
>
> Yes.
>
>>> So the SoC is always pulling
>>> that line up or down, and the panel is also pulling it up or down when
>>> it's sending data. I'm no HW guy but that sounds quite bad =).
>
> Yes, that is the strange thing with this protocol. It does a direction reversal
> after some time. I.e. the panel switches its input to output and the SoC has
> to be fast enough to do the same. Therefore, we have one output GPIO
> (protected by a resistor) and a separate input GPIO.
>
> Sometimes interfacing hardware is indeed strange.
>
>>>
>>> I've never written or studied a bitbanging driver, but shouldn't there
>>> be just one gpio used on the SoC for DIN, and it would be set to either
>>> output or input mode, depending on if we are reading or writing?
>>
>> Back in the OpenMoko days we used the panel in normal 4-wire SPI mode with
>> the GPIO bitbang SPI master. The bitbang code in this driver also looks like
>> normal 4 wire.
>
>
> Thanks for this hint!
>
> Maybe using a standard 4-wire SPI simply works if we only write data and
> make sure the panel is not sending anything...

According to the datasheet the the panel as a dedicated dout pin. Maybe you did 
not connect it in your design, which means you won't be able to read any data 
from the panel at all.

Also your custom bitbang code looks a bit strange:

	gpio_set_value(data->dout_gpio, 1);
	if (gpio_get_value(data->din_gpio) = 0)
		return 1;

You set the state on the dout pin and then read the din pin, if both do not 
match you abort with an error. This suggest that, for whatever reason, you feed 
the dout pin back into the din pin in your design. Btw. this is also the only 
place where din is used in your driver.

- Lars

^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Tomi Valkeinen @ 2013-10-11  4:41 UTC (permalink / raw)
  To: Lars-Peter Clausen, Dr. H. Nikolaus Schaller
  Cc: Marek Belisko, plagnioj, linux-kernel, linux-omap, linux-fbdev
In-Reply-To: <5256F8DF.2020801@metafoo.de>

[-- Attachment #1: Type: text/plain, Size: 1146 bytes --]

On 10/10/13 21:58, Lars-Peter Clausen wrote:

> According to the datasheet the the panel as a dedicated dout pin. Maybe
> you did not connect it in your design, which means you won't be able to
> read any data from the panel at all.

I don't see a dedicated dout in the datasheet...
http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf

> Also your custom bitbang code looks a bit strange:
> 
>     gpio_set_value(data->dout_gpio, 1);
>     if (gpio_get_value(data->din_gpio) == 0)
>         return 1;
> 
> You set the state on the dout pin and then read the din pin, if both do
> not match you abort with an error. This suggest that, for whatever
> reason, you feed the dout pin back into the din pin in your design. Btw.
> this is also the only place where din is used in your driver.

Yes, he said the single "Serial interface data input/output" pin is
connected to both din and dout on the SoC. I guess the purpose of that
gpio_get_value() is to ensure that the panel is not pulling the line
when the SoC is writing to it. Not that I really understand how that can
work, but I'm not a HW guy =).

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: linux-next: manual merge of the omap_dss2 tree
From: Tomi Valkeinen @ 2013-10-11  5:18 UTC (permalink / raw)
  To: Mark Brown, Jean-Christophe PLAGNIOL-VILLARD
  Cc: Archit Taneja, linux-omap, linux-fbdev, linux-next, linux-kernel,
	Thierry Reding
In-Reply-To: <20131010175020.GS21581@sirena.org.uk>

[-- Attachment #1: Type: text/plain, Size: 809 bytes --]

Hi Mark, Jean-Christophe,

On 10/10/13 20:50, Mark Brown wrote:
> Today's linux-next merge of the omap_dss2 tree got conflicts in
> drivers/video/omap2/dss/hdmi4_core.[ch] caused by ef26958a (omapdss:
> HDMI: Rename hdmi driver files to nicer names) interacting with a range
> of commits from Ricardo Neri in the fbdev tree and possibly some other
> stuff.  git is unfortunately not giving me a useful diff right now :/

Jean-Christophe has slightly different versions of my patches in his
fbdev for-next branch, so they conflict with my updated versions.

Jean-Christophe, I expected this to happen with the current way of you
having a copy of my for-next branch in yours. Can I now take your
atmel_lcdfb patches to my for-next, and you'll remove all patches from
your for-next?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH v3 1/3] video: xilinxfb: Use standard variable name convention
From: Tomi Valkeinen @ 2013-10-11  5:58 UTC (permalink / raw)
  To: Michal Simek
  Cc: linux-kernel, monstr, joe, Jean-Christophe Plagniol-Villard,
	Grant Likely, Rob Herring, linux-fbdev, devicetree
In-Reply-To: <631df9a44b366af4129d00f1d4e1d3baad7d4903.1381386616.git.michal.simek@xilinx.com>

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

On 10/10/13 09:30, Michal Simek wrote:
> s/op/pdev/ in xilinxfb_of_probe().
> No functional chagnes.
> 
> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
> ---
> Changes in v3: None
> Changes in v2: None
> 
>  drivers/video/xilinxfb.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)

Thanks, queued this series for 3.13.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Tomi Valkeinen @ 2013-10-11  6:37 UTC (permalink / raw)
  To: Andrzej Hajda, Laurent Pinchart
  Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
	Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
	Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
	Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <52556370.1050102@samsung.com>

[-- Attachment #1: Type: text/plain, Size: 5336 bytes --]

On 09/10/13 17:08, Andrzej Hajda wrote:

> As I have adopted existing internal driver for MIPI-DSI bus, I did not
> take too much
> care for DT. You are right, 'bta-timeout' is a configuration parameter
> (however its
> minimal value is determined by characteristic of the DSI-slave). On the
> other
> side currently there is no good place for such configuration parameters
> AFAIK.

The minimum bta-timeout should be deducable from the DSI bus speed,
shouldn't it? Thus there's no need to define it anywhere.

>> - enable_hs and enable_te, used to enable/disable HS mode and
>> tearing-elimination
> 
> It seems there should be a way to synchronize TE signal with panel,
> in case signal is provided only to dsi-master. Some callback I suppose?
> Or transfer synchronization should be done by dsi-master.

Hmm, can you explain a bit what you mean?

Do you mean that the panel driver should get a callback when DSI TE
trigger happens?

On OMAP, when using DSI TE trigger, the dsi-master does it all. So the
panel driver just calls update() on the dsi-master, and then the
dsi-master will wait for TE, and then start the transfer. There's also a
callback to the panel driver when the transfer has completed.

>> - set_max_rx_packet_size, used to configure the max rx packet size.
> Similar callbacks should be added to mipi-dsi-bus ops as well, to
> make it complete/generic.

Do you mean the same calls should exist both in the mipi-dbi-bus ops and
on the video ops? If they are called with different values, which one
"wins"?

>> http://article.gmane.org/gmane.comp.video.dri.devel/90651
>> http://article.gmane.org/gmane.comp.video.dri.devel/91269
>> http://article.gmane.org/gmane.comp.video.dri.devel/91272
>>
>> I still think that it's best to consider DSI and DBI as a video bus (not
>> as a separate video bus and a control bus), and provide the packet
>> transfer methods as part of the video ops.
> I have read all posts regarding this issue and currently I tend
> to solution where CDF is used to model only video streams,
> with control bus implemented in different framework.
> The only concerns I have if we should use Linux bus for that.

Ok. I have many other concerns, as I've expressed in the mails =). I
still don't see how it could work. So I'd very much like to see a more
detailed explanation how the separate control & video bus approach would
deal with different scenarios.

Let's consider a DSI-to-HDMI encoder chip. Version A of the chip is
controlled via DSI, version B is controlled via i2c. As the output of
the chip goes to HDMI connector, the DSI bus speed needs to be set
according to the resolution of the HDMI monitor.

So, with version A, the encoder driver would have some kind of pointers
to ctrl_ops and video_ops (or, pointers to dsi_bus instance and
video_bus instance), right? The ctrl_ops would need to have ops like
set_bus_speed, enable_hs, etc, to configure the DSI bus.

When the encoder driver is started, it'd probably set some safe bus
speed, configure the encoder a bit, read the EDID, enable HS,
re-configure the bus speed to match the monitor's video mode, configure
the encoder, and at last enable the video stream.

Version B would have i2c_client and video_ops. When the driver starts,
it'd  probably do the same things as above, except the control messages
would go through i2c. That means that setting the bus speed, enabling
HS, etc, would happen through video_ops, as the i2c side has no
knowledge of the DSI side, right? Would there be identical ops on both
DSI ctrl and video ops?

That sounds very bad. What am I missing here? How would it work?

And, if we want to separate the video and control, I see no reason to
explicitly require the video side to be present. I.e. we could as well
have a DSI peripheral that has only the control bus used. How would that
reflect to, say, the DT presentation? Say, if we have a version A of the
encoder, we could have DT data like this (just a rough example):

soc-dsi {
	encoder {
		input: endpoint {
			remote-endpoint = <&soc-dsi-ep>;
			/* configuration for the DSI lanes */
			dsi-lanes = <0 1 2 3 4 5>;
		};
	};
};

So the encoder would be places inside the SoC's DSI node, similar to how
an i2c device would be placed inside SoC's i2c node. DSI configuration
would be inside the video endpoint data.

Version B would be almost the same:

&i2c0 {
	encoder {
		input: endpoint {
			remote-endpoint = <&soc-dsi-ep>;
			/* configuration for the DSI lanes */
			dsi-lanes = <0 1 2 3 4 5>;
		};
	};
};

Now, how would the video-bus-less device be defined? It'd be inside the
soc-dsi node, that's clear. Where would the DSI lane configuration be?
Not inside 'endpoint' node, as that's for video and wouldn't exist in
this case. Would we have the same lane configuration in two places, once
for video and once for control?

I agree that having DSI/DBI control and video separated would be
elegant. But I'd like to hear what is the technical benefit of that? At
least to me it's clearly more complex to separate them than to keep them
together (to the extent that I don't yet see how it is even possible),
so there must be a good reason for the separation. I don't understand
that reason. What is it?

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Lars-Peter Clausen @ 2013-10-11  7:08 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Dr. H. Nikolaus Schaller, Marek Belisko, plagnioj, linux-kernel,
	linux-omap, linux-fbdev
In-Reply-To: <5257815F.5070803@ti.com>

On 10/11/2013 06:41 AM, Tomi Valkeinen wrote:
> On 10/10/13 21:58, Lars-Peter Clausen wrote:
>
>> According to the datasheet the the panel as a dedicated dout pin. Maybe
>> you did not connect it in your design, which means you won't be able to
>> read any data from the panel at all.
>
> I don't see a dedicated dout in the datasheet...
> http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf

Hm, ok, looks like the display controller used in the panel (the jbt6k74) has 
separate DIN and DOUT, but the panel only has one pin. But the datasheet for 
the panel is not exactly clear on whether it's DIN pin is both the DIN and DOUT 
pin from the controller or, just DIN and DOUT not being connected at all.

>
>> Also your custom bitbang code looks a bit strange:
>>
>>      gpio_set_value(data->dout_gpio, 1);
>>      if (gpio_get_value(data->din_gpio) = 0)
>>          return 1;
>>
>> You set the state on the dout pin and then read the din pin, if both do
>> not match you abort with an error. This suggest that, for whatever
>> reason, you feed the dout pin back into the din pin in your design. Btw.
>> this is also the only place where din is used in your driver.
>
> Yes, he said the single "Serial interface data input/output" pin is
> connected to both din and dout on the SoC. I guess the purpose of that
> gpio_get_value() is to ensure that the panel is not pulling the line
> when the SoC is writing to it. Not that I really understand how that can
> work, but I'm not a HW guy =).

Hm, ok. I don't think the panel will ever actively drive the line. So in my 
opinion using either the McBSP SPI master or the GPIO bitbang SPI master should 
work just fine. You just wouldn't be able to read any register from the device. 
But the driver is not attempting to do this, so it should be fine.

- Lars

^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Dr. H. Nikolaus Schaller @ 2013-10-11  7:29 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Lars-Peter Clausen, Marek Belisko, plagnioj, linux-kernel,
	linux-omap, linux-fbdev
In-Reply-To: <5257815F.5070803@ti.com>

Hi all,

Am 11.10.2013 um 06:41 schrieb Tomi Valkeinen:

> On 10/10/13 21:58, Lars-Peter Clausen wrote:
> 
>> According to the datasheet the the panel as a dedicated dout pin. Maybe
>> you did not connect it in your design, which means you won't be able to
>> read any data from the panel at all.
> 
> I don't see a dedicated dout in the datasheet...
> http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf
> 
>> Also your custom bitbang code looks a bit strange:
>> 
>>    gpio_set_value(data->dout_gpio, 1);
>>    if (gpio_get_value(data->din_gpio) = 0)
>>        return 1;
>> 
>> You set the state on the dout pin and then read the din pin, if both do
>> not match you abort with an error. This suggest that, for whatever
>> reason,

Detecting hardware failure.

If the panel is not connected or broken (short circuit) reading back
may fail and this can help to detect a hardware failure.

>> you feed the dout pin back into the din pin in your design. Btw.
>> this is also the only place where din is used in your driver.

Ok, I see the issue with that. It assumes this type of feedback which does
not necessarily exist. I.e. if someone else doesn't have this feedback
line the driver will not work.

So a more general solution should work with any setup, even if no
din gpio has been defined. I.e. do this test only if the din_gpio exists.

The main reason that we don't use din_gpio elsewhere is that we have
no jbt_read() function because we lack information what we can
read from the panel controller chip - and have not seen a use-case
for that.

And thanks to your hint, I think either returning 1 is wrong (should be
some -EIO or similar). Or the return r; at the end of td028ttec1_panel_enable
is wrong (should be return rc ? -EIO : 0; ).

We will check that.

> Yes, he said the single "Serial interface data input/output" pin is
> connected to both din and dout on the SoC.

I have found two public links which may describe what we do:

The panel data sheet [1], Section 9 describes the interface as
"3 wire serial interface".

It sometimes calls the data lines DIN and DOUT and describes them
separately, i.e. like a 4 wire interface.

So I guess the jbt6k chip has indeed 4 wires, but (as you said),

din/dout are lines are tied together on the panel side to save
(share) one wire in the flex cable.
 
[2] shows in figure 2 an example how to connect a 3-wire interface
to a 4-wire SPI SoC. This is the way we have done it.


>  I guess the purpose of that
> gpio_get_value() is to ensure that the panel is not pulling the line
> when the SoC is writing to it. Not that I really understand how that can
> work, but I'm not a HW guy =).

Finally, I have checked one detail which rules out to use a standard
SPI driver for OMAP on our board. The reason is that we have a McBSP
port of the OMAP3 and not a McSPI. So we have to run this "3 wire
serial interface protocol" by bitbanging.

IMHO, the most flexible way to hook up the panel (and driver) to arbitrary
hardware is by using GPIOs and not having the panel driver restrict to a
SPI interface.

And having the bitbanging encapsulated in the driver itself (instead on
relying on some spi-bitbang) makes it less dependent on changes
somewhere else. I.e. the driver module is indeed a module.

Marek is preparing an updated patch for further discussion.

BR,
Nikolaus


[1]: http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf
[2]: http://www.totalphase.com/support/kb/10059/


^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Dr. H. Nikolaus Schaller @ 2013-10-11  7:42 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Tomi Valkeinen, Marek Belisko, plagnioj, linux-kernel, linux-omap,
	linux-fbdev
In-Reply-To: <5257A402.5030806@metafoo.de>

Hi Lars-Peter,
ah, I didn't see your mail while writing mine - so some overlap.

Am 11.10.2013 um 09:08 schrieb Lars-Peter Clausen:

> On 10/11/2013 06:41 AM, Tomi Valkeinen wrote:
>> On 10/10/13 21:58, Lars-Peter Clausen wrote:
>> 
>>> According to the datasheet the the panel as a dedicated dout pin. Maybe
>>> you did not connect it in your design, which means you won't be able to
>>> read any data from the panel at all.
>> 
>> I don't see a dedicated dout in the datasheet...
>> http://dl.wolfgang-draxinger.net/openmoko/doc/TD028TTEC1.pdf
> 
> Hm, ok, looks like the display controller used in the panel (the jbt6k74) has separate DIN and DOUT, but the panel only has one pin.

Yes

> But the datasheet for the panel is not exactly clear

Yes

> on whether it's DIN pin is both the DIN and DOUT pin from the controller or, just DIN and DOUT not being connected at all.

I think they have a 4-wire controller in the panel and tied the lines together.
This works if they properly control the data direction.
There are some read commands where this direction reversal happens.

> 
>> 
>>> Also your custom bitbang code looks a bit strange:
>>> 
>>>     gpio_set_value(data->dout_gpio, 1);
>>>     if (gpio_get_value(data->din_gpio) = 0)
>>>         return 1;
>>> 
>>> You set the state on the dout pin and then read the din pin, if both do
>>> not match you abort with an error. This suggest that, for whatever
>>> reason, you feed the dout pin back into the din pin in your design. Btw.
>>> this is also the only place where din is used in your driver.
>> 
>> Yes, he said the single "Serial interface data input/output" pin is
>> connected to both din and dout on the SoC. I guess the purpose of that
>> gpio_get_value() is to ensure that the panel is not pulling the line
>> when the SoC is writing to it. Not that I really understand how that can
>> work, but I'm not a HW guy =).
> 
> Hm, ok. I don't think the panel will ever actively drive the line.

Well it will drive it if we issue some read command (but I have no information
about these commands).

> So in my opinion using either the McBSP SPI master or the GPIO bitbang SPI master should work just fine. You just wouldn't be able to read any register from the device. But the driver is not attempting to do this, so it should be fine.

I am not sure if there is a SPI driver for a McBSP port [1]? And to make that
work (reliably) and tested it might need a lot of work for us. At least I think
such a change (e.g. setting up clock polarity etc.) is not done in some minutes.
And the only feedback we have from the panel is "does not work"/"works". I.e.
if we are not lucky that it works immediately we have no real means to debug.

IMHO it also gives more flexibility to board designers to choose GPIOs instead
of enforcing some SPI interface by the driver (and encapsulate this arguable
protocol in the driver). Maybe some board has 3 spare GPIOs but neither
McBSPs nor McSPIs available.

BR,
Nikolaus

http://lists.infradead.org/pipermail/linux-arm-kernel/2011-June/053064.html


^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Tomi Valkeinen @ 2013-10-11  8:17 UTC (permalink / raw)
  To: Dr. H. Nikolaus Schaller, Lars-Peter Clausen
  Cc: Marek Belisko, plagnioj, linux-kernel, linux-omap, linux-fbdev
In-Reply-To: <5570E6D7-4F66-40BE-A50C-6CCA8074BD83@goldelico.com>

[-- Attachment #1: Type: text/plain, Size: 1688 bytes --]

On 11/10/13 10:42, Dr. H. Nikolaus Schaller wrote:

> I am not sure if there is a SPI driver for a McBSP port [1]? And to make that
> work (reliably) and tested it might need a lot of work for us. At least I think
> such a change (e.g. setting up clock polarity etc.) is not done in some minutes.
> And the only feedback we have from the panel is "does not work"/"works". I.e.
> if we are not lucky that it works immediately we have no real means to debug.
> 
> IMHO it also gives more flexibility to board designers to choose GPIOs instead
> of enforcing some SPI interface by the driver (and encapsulate this arguable
> protocol in the driver). Maybe some board has 3 spare GPIOs but neither
> McBSPs nor McSPIs available.

This has been an interesting thread, I've learnt a lot =).

I still think the panel driver should not handle this, but there should
be a separate spi bitbang driver for it.

I understand you're not enthusiastic going that way, as the current
version works for you. However, when using DT, we need to think how to
represent the hardware in the device tree data, and it has to be right
from the beginning.

That's why I won't allow representing this panel as having 4 gpios in
the DT data, because that is not correct. The panel has 3 pins. But
then, the panel does allow reading, which could be implemented using 4
gpios as you have done. This data should be in the spi-bitbang data, and
the panel should just use the standard SPI framework.

Using SPI framework does not mean you should use McBSP or McSPI. It's up
to you how the 3-wire SPI is implemented on the SoC side, the panel
would just work in all the cases.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Belisko Marek @ 2013-10-11  8:59 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Dr. H. Nikolaus Schaller, Lars-Peter Clausen,
	Jean-Christophe PLAGNIOL-VILLARD, LKML,
	linux-omap@vger.kernel.org, linux-fbdev
In-Reply-To: <5257B420.10808@ti.com>

Hi Tomi,

On Fri, Oct 11, 2013 at 10:17 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> On 11/10/13 10:42, Dr. H. Nikolaus Schaller wrote:
>
>> I am not sure if there is a SPI driver for a McBSP port [1]? And to make that
>> work (reliably) and tested it might need a lot of work for us. At least I think
>> such a change (e.g. setting up clock polarity etc.) is not done in some minutes.
>> And the only feedback we have from the panel is "does not work"/"works". I.e.
>> if we are not lucky that it works immediately we have no real means to debug.
>>
>> IMHO it also gives more flexibility to board designers to choose GPIOs instead
>> of enforcing some SPI interface by the driver (and encapsulate this arguable
>> protocol in the driver). Maybe some board has 3 spare GPIOs but neither
>> McBSPs nor McSPIs available.
>
> This has been an interesting thread, I've learnt a lot =).
>
> I still think the panel driver should not handle this, but there should
> be a separate spi bitbang driver for it.
>
> I understand you're not enthusiastic going that way, as the current
> version works for you. However, when using DT, we need to think how to
> represent the hardware in the device tree data, and it has to be right
> from the beginning.
>
> That's why I won't allow representing this panel as having 4 gpios in
> the DT data, because that is not correct. The panel has 3 pins. But
> then, the panel does allow reading, which could be implemented using 4
> gpios as you have done. This data should be in the spi-bitbang data, and
> the panel should just use the standard SPI framework.
I disagree. There are different drivers which pass in platform data
gpios (encoder-tfp410.c or encoder-tpd12s015.c)
and those must be covered by DT then. I cannot see problem why to have
for td028 panel 3 or 4 gpios defined in DT.
>
> Using SPI framework does not mean you should use McBSP or McSPI. It's up
> to you how the 3-wire SPI is implemented on the SoC side, the panel
> would just work in all the cases.
>
>  Tomi
>
>

BR,

marek

-- 
as simple and primitive as possible
-------------------------------------------------
Marek Belisko - OPEN-NANDRA
Freelance Developer

Ruska Nova Ves 219 | Presov, 08005 Slovak Republic
Tel: +421 915 052 184
skype: marekwhite
twitter: #opennandra
web: http://open-nandra.com

^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Tomi Valkeinen @ 2013-10-11  9:04 UTC (permalink / raw)
  To: Belisko Marek
  Cc: Dr. H. Nikolaus Schaller, Lars-Peter Clausen,
	Jean-Christophe PLAGNIOL-VILLARD, LKML,
	linux-omap@vger.kernel.org, linux-fbdev
In-Reply-To: <CAAfyv36HSb4FOxaYnqcwBusqSuhtVnT0gEAoGgcW_rBkhWhf0A@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 1161 bytes --]

On 11/10/13 11:59, Belisko Marek wrote:

>> That's why I won't allow representing this panel as having 4 gpios in
>> the DT data, because that is not correct. The panel has 3 pins. But
>> then, the panel does allow reading, which could be implemented using 4
>> gpios as you have done. This data should be in the spi-bitbang data, and
>> the panel should just use the standard SPI framework.
> I disagree. There are different drivers which pass in platform data
> gpios (encoder-tfp410.c or encoder-tpd12s015.c)
> and those must be covered by DT then. I cannot see problem why to have
> for td028 panel 3 or 4 gpios defined in DT.

Yes, they are plain GPIOs, and defined in the spec for the respective
chip. There's no alternative to how they could be represented.

Here, with the td028, the spec speaks of 3 pins for the serial bus. Not
4. So there cannot be 4 gpios defined in the panel's data, that just
doesn't make sense.

Additionally, what we have here are not "normal" gpios, but pins for a
serial bus, 3-wire SPI. If the panel data specifies the gpios, then it's
not possible to use real SPI hardware with the panel.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Lars-Peter Clausen @ 2013-10-11  9:06 UTC (permalink / raw)
  To: Belisko Marek
  Cc: Tomi Valkeinen, Dr. H. Nikolaus Schaller,
	Jean-Christophe PLAGNIOL-VILLARD, LKML,
	linux-omap@vger.kernel.org, linux-fbdev
In-Reply-To: <CAAfyv36HSb4FOxaYnqcwBusqSuhtVnT0gEAoGgcW_rBkhWhf0A@mail.gmail.com>

On 10/11/2013 10:59 AM, Belisko Marek wrote:
> Hi Tomi,
> 
> On Fri, Oct 11, 2013 at 10:17 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>> On 11/10/13 10:42, Dr. H. Nikolaus Schaller wrote:
>>
>>> I am not sure if there is a SPI driver for a McBSP port [1]? And to make that
>>> work (reliably) and tested it might need a lot of work for us. At least I think
>>> such a change (e.g. setting up clock polarity etc.) is not done in some minutes.
>>> And the only feedback we have from the panel is "does not work"/"works". I.e.
>>> if we are not lucky that it works immediately we have no real means to debug.
>>>
>>> IMHO it also gives more flexibility to board designers to choose GPIOs instead
>>> of enforcing some SPI interface by the driver (and encapsulate this arguable
>>> protocol in the driver). Maybe some board has 3 spare GPIOs but neither
>>> McBSPs nor McSPIs available.
>>
>> This has been an interesting thread, I've learnt a lot =).
>>
>> I still think the panel driver should not handle this, but there should
>> be a separate spi bitbang driver for it.
>>
>> I understand you're not enthusiastic going that way, as the current
>> version works for you. However, when using DT, we need to think how to
>> represent the hardware in the device tree data, and it has to be right
>> from the beginning.
>>
>> That's why I won't allow representing this panel as having 4 gpios in
>> the DT data, because that is not correct. The panel has 3 pins. But
>> then, the panel does allow reading, which could be implemented using 4
>> gpios as you have done. This data should be in the spi-bitbang data, and
>> the panel should just use the standard SPI framework.
> I disagree. There are different drivers which pass in platform data
> gpios (encoder-tfp410.c or encoder-tpd12s015.c)
> and those must be covered by DT then. I cannot see problem why to have
> for td028 panel 3 or 4 gpios defined in DT.

The problem is not representing it in the devicetree, but representing it
correctly. This is a SPI slave device, hence it should be presented in the
devicetree as a SPI slave device and not as a platform device with 4 GPIOs.

- Lars


^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Dr. H. Nikolaus Schaller @ 2013-10-11  9:50 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Belisko Marek, Tomi Valkeinen, Jean-Christophe PLAGNIOL-VILLARD,
	LKML, linux-omap@vger.kernel.org, linux-fbdev
In-Reply-To: <5257BFA3.9090202@metafoo.de>

Hi all,

Am 11.10.2013 um 11:06 schrieb Lars-Peter Clausen:

> On 10/11/2013 10:59 AM, Belisko Marek wrote:
>> Hi Tomi,
>> 
>> On Fri, Oct 11, 2013 at 10:17 AM, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> On 11/10/13 10:42, Dr. H. Nikolaus Schaller wrote:
>>> 
>>>> I am not sure if there is a SPI driver for a McBSP port [1]? And to make that
>>>> work (reliably) and tested it might need a lot of work for us. At least I think
>>>> such a change (e.g. setting up clock polarity etc.) is not done in some minutes.
>>>> And the only feedback we have from the panel is "does not work"/"works". I.e.
>>>> if we are not lucky that it works immediately we have no real means to debug.
>>>> 
>>>> IMHO it also gives more flexibility to board designers to choose GPIOs instead
>>>> of enforcing some SPI interface by the driver (and encapsulate this arguable
>>>> protocol in the driver). Maybe some board has 3 spare GPIOs but neither
>>>> McBSPs nor McSPIs available.
>>> 
>>> This has been an interesting thread, I've learnt a lot =).
>>> 
>>> I still think the panel driver should not handle this, but there should
>>> be a separate spi bitbang driver for it.
>>> 
>>> I understand you're not enthusiastic going that way, as the current
>>> version works for you. However, when using DT, we need to think how to
>>> represent the hardware in the device tree data, and it has to be right
>>> from the beginning.
>>> 
>>> That's why I won't allow representing this panel as having 4 gpios in
>>> the DT data, because that is not correct. The panel has 3 pins. But
>>> then, the panel does allow reading, which could be implemented using 4
>>> gpios as you have done. This data should be in the spi-bitbang data, and
>>> the panel should just use the standard SPI framework.
>> I disagree. There are different drivers which pass in platform data
>> gpios (encoder-tfp410.c or encoder-tpd12s015.c)
>> and those must be covered by DT then. I cannot see problem why to have
>> for td028 panel 3 or 4 gpios defined in DT.
> 
> The problem is not representing it in the devicetree, but representing it
> correctly. This is a SPI slave device, hence it should be presented in the
> devicetree as a SPI slave device and not as a platform device with 4 GPIOs.

Hm. Is this a SPI or does it just look like one? Or is it some - otherwise
unknown - "3 wire serial interface". Or is it a "3(+1) GPIO slave device"?
I am still not sure about this.

If we really want to do it correctly, we may have to write a driver for that
special serial protocol as well. If it turns out that we can't mis-use and tweak
it into a standard SPI driver with bit-bang backend.

I simply fear that we get dependencies with the SPI subsystem and have
to test, debug and maintain it. Maintaining the GPIO thing we currently have
is easy.

What would be against taking the GPIO approach first and then upgrade as soon
as someone raises his/her finger that he/she wants to really interface this display
differently and is not happy with the 3/4 GPIOs? Either they come up with a patch
or contact the driver author (=me).

BR,
Nikolaus


^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Tomi Valkeinen @ 2013-10-11 10:09 UTC (permalink / raw)
  To: Dr. H. Nikolaus Schaller, Lars-Peter Clausen
  Cc: Belisko Marek, Jean-Christophe PLAGNIOL-VILLARD, LKML,
	linux-omap@vger.kernel.org, linux-fbdev
In-Reply-To: <B8F44C8A-362C-4E78-BE91-D9C0783B998B@goldelico.com>

[-- Attachment #1: Type: text/plain, Size: 2296 bytes --]

On 11/10/13 12:50, Dr. H. Nikolaus Schaller wrote:

> Hm. Is this a SPI or does it just look like one? Or is it some - otherwise
> unknown - "3 wire serial interface". Or is it a "3(+1) GPIO slave device"?
> I am still not sure about this.

Lars-Peter said "Back in the OpenMoko days we used the panel in normal
4-wire SPI mode with the GPIO bitbang SPI master."

I don't know much about SPI, so I can't answer to that. If the serial
bus is indeed not any kind of more or less standard SPI version, but
really a custom bus for this controller, then the case is a bit unclear.

> If we really want to do it correctly, we may have to write a driver for that
> special serial protocol as well. If it turns out that we can't mis-use and tweak
> it into a standard SPI driver with bit-bang backend.
> 
> I simply fear that we get dependencies with the SPI subsystem and have
> to test, debug and maintain it. Maintaining the GPIO thing we currently have
> is easy.
> 
> What would be against taking the GPIO approach first and then upgrade as soon
> as someone raises his/her finger that he/she wants to really interface this display
> differently and is not happy with the 3/4 GPIOs? Either they come up with a patch
> or contact the driver author (=me).

I don't have anything against that as long as we use only platform data.

But DT data is not an in-kernel API, it's an external API. Once we
define that the DT data for this panel is something, that's it, we
should stick to it. Of course, we can build compatibility layers for old
DT data, but I would avoid that if at all possible.

If we now create the DT data with gpios, and the panel as platform
device, it'd be rather nasty change to make it a child of an spi bus. (I
presume, I have never made such a change).

And, as the gpios and platform device approach is clearly wrong way to
describe the hardware, I'm quite against using that description in the
DT data.

That said, one option is to describe the hardware correctly in the DT
data, but have a platform device for the panel, with panel driver doing
the bitbanging. In that case it is possible to update the system to use
SPI framework if needed, without changing the DT data. However, I'm not
sure how easy that would be.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH] omapdss: Add new panel driver for Topolly td028ttec1 LCD.
From: Dr. H. Nikolaus Schaller @ 2013-10-11 11:03 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Lars-Peter Clausen, Belisko Marek,
	Jean-Christophe PLAGNIOL-VILLARD, LKML,
	linux-omap@vger.kernel.org, linux-fbdev
In-Reply-To: <5257CE55.3090902@ti.com>

Hi Tomi,

Am 11.10.2013 um 12:09 schrieb Tomi Valkeinen:

> On 11/10/13 12:50, Dr. H. Nikolaus Schaller wrote:
> 
>> Hm. Is this a SPI or does it just look like one? Or is it some - otherwise
>> unknown - "3 wire serial interface". Or is it a "3(+1) GPIO slave device"?
>> I am still not sure about this.
> 
> Lars-Peter said "Back in the OpenMoko days we used the panel in normal
> 4-wire SPI mode with the GPIO bitbang SPI master."
> 
> I don't know much about SPI, so I can't answer to that. If the serial
> bus is indeed not any kind of more or less standard SPI version, but
> really a custom bus for this controller, then the case is a bit unclear.

I have thought over lunch time that it is worth to look into how the Openmoko
did physically hook up the display and if parts of that code (it was for 2.6.26
or so and for a Samsung SoC) is understandable and reusable (e.g. hints
how to set up the board file for a bitbang SPI on OMAP3 - we have never
done this before).

> 
>> If we really want to do it correctly, we may have to write a driver for that
>> special serial protocol as well. If it turns out that we can't mis-use and tweak
>> it into a standard SPI driver with bit-bang backend.
>> 
>> I simply fear that we get dependencies with the SPI subsystem and have
>> to test, debug and maintain it. Maintaining the GPIO thing we currently have
>> is easy.
>> 
>> What would be against taking the GPIO approach first and then upgrade as soon
>> as someone raises his/her finger that he/she wants to really interface this display
>> differently and is not happy with the 3/4 GPIOs? Either they come up with a patch
>> or contact the driver author (=me).
> 
> I don't have anything against that as long as we use only platform data.
> 
> But DT data is not an in-kernel API, it's an external API. Once we
> define that the DT data for this panel is something, that's it, we
> should stick to it. Of course, we can build compatibility layers for old
> DT data, but I would avoid that if at all possible.

Ah, I see. I already think that using the DT makes such things not easier
but more difficult - but I am not at all familiar with it.

> If we now create the DT data with gpios, and the panel as platform
> device, it'd be rather nasty change to make it a child of an spi bus. (I
> presume, I have never made such a change).
> 
> And, as the gpios and platform device approach is clearly wrong way to
> describe the hardware, I'm quite against using that description in the
> DT data.

Since we are not familiar with DT yet, it is difficult to see the consequences
of such a step.

> That said, one option is to describe the hardware correctly in the DT
> data, but have a platform device for the panel, with panel driver doing
> the bitbanging. In that case it is possible to update the system to use
> SPI framework if needed, without changing the DT data. However, I'm not
> sure how easy that would be.

Sounds quite complex, indeed.

So our next step will be to look into the GTA02 SPI thing to get more knowlegde
about thier solution.

BR,
Nikolaus


^ permalink raw reply

* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Andrzej Hajda @ 2013-10-11 11:19 UTC (permalink / raw)
  To: Tomi Valkeinen, Laurent Pinchart
  Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
	Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
	Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
	Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <52579CB2.8050601@ti.com>

On 10/11/2013 08:37 AM, Tomi Valkeinen wrote:
> On 09/10/13 17:08, Andrzej Hajda wrote:
>
>> As I have adopted existing internal driver for MIPI-DSI bus, I did not
>> take too much
>> care for DT. You are right, 'bta-timeout' is a configuration parameter
>> (however its
>> minimal value is determined by characteristic of the DSI-slave). On the
>> other
>> side currently there is no good place for such configuration parameters
>> AFAIK.
> The minimum bta-timeout should be deducable from the DSI bus speed,
> shouldn't it? Thus there's no need to define it anywhere.
Hmm, specification says "This specified period shall be longer then
the maximum possible turnaround delay for the unit to which the
turnaround request was sent".
>
>>> - enable_hs and enable_te, used to enable/disable HS mode and
>>> tearing-elimination
>> It seems there should be a way to synchronize TE signal with panel,
>> in case signal is provided only to dsi-master. Some callback I suppose?
>> Or transfer synchronization should be done by dsi-master.
> Hmm, can you explain a bit what you mean?
>
> Do you mean that the panel driver should get a callback when DSI TE
> trigger happens?
>
> On OMAP, when using DSI TE trigger, the dsi-master does it all. So the
> panel driver just calls update() on the dsi-master, and then the
> dsi-master will wait for TE, and then start the transfer. There's also a
> callback to the panel driver when the transfer has completed.
Yes I though about a callback, but approach with DSI-master taking care
of synchronization in fact better fits to exynos-dsi and I suspect to
omap also.
>
>>> - set_max_rx_packet_size, used to configure the max rx packet size.
>> Similar callbacks should be added to mipi-dsi-bus ops as well, to
>> make it complete/generic.
> Do you mean the same calls should exist both in the mipi-dbi-bus ops and
> on the video ops? If they are called with different values, which one
> "wins"?
No, I meant that if mipi-dbi-bus want to be complete it should have
similar ops.
I did not think about scenario with two overlaping APIs.
>
>>> http://article.gmane.org/gmane.comp.video.dri.devel/90651
>>> http://article.gmane.org/gmane.comp.video.dri.devel/91269
>>> http://article.gmane.org/gmane.comp.video.dri.devel/91272
>>>
>>> I still think that it's best to consider DSI and DBI as a video bus (not
>>> as a separate video bus and a control bus), and provide the packet
>>> transfer methods as part of the video ops.
>> I have read all posts regarding this issue and currently I tend
>> to solution where CDF is used to model only video streams,
>> with control bus implemented in different framework.
>> The only concerns I have if we should use Linux bus for that.
> Ok. I have many other concerns, as I've expressed in the mails =). I
> still don't see how it could work. So I'd very much like to see a more
> detailed explanation how the separate control & video bus approach would
> deal with different scenarios.
>
> Let's consider a DSI-to-HDMI encoder chip. Version A of the chip is
> controlled via DSI, version B is controlled via i2c. As the output of
> the chip goes to HDMI connector, the DSI bus speed needs to be set
> according to the resolution of the HDMI monitor.
>
> So, with version A, the encoder driver would have some kind of pointers
> to ctrl_ops and video_ops (or, pointers to dsi_bus instance and
> video_bus instance), right? The ctrl_ops would need to have ops like
> set_bus_speed, enable_hs, etc, to configure the DSI bus.
>
> When the encoder driver is started, it'd probably set some safe bus
> speed, configure the encoder a bit, read the EDID, enable HS,
> re-configure the bus speed to match the monitor's video mode, configure
> the encoder, and at last enable the video stream.
>
> Version B would have i2c_client and video_ops. When the driver starts,
> it'd  probably do the same things as above, except the control messages
> would go through i2c. That means that setting the bus speed, enabling
> HS, etc, would happen through video_ops, as the i2c side has no
> knowledge of the DSI side, right? Would there be identical ops on both
> DSI ctrl and video ops?
>
> That sounds very bad. What am I missing here? How would it work?
If I undrestand correctly you think about CDF topology like below:

DispContr(SoC) ---> DSI-master(SoC) ---> encoder(DSI or I2C)

But I think with mipi-dsi-bus topology could look like:

DispContr(SoC) ---> encoder(DSI or I2C)

DSI-master will not have its own entity, in the graph it could be
represented
by the link(--->), as it really does not process the video, only
transports it.

In case of version A I think everything is clear.
In case of version B it does not seems so nice at the first sight, but
still seems quite straightforward to me - special plink in encoder's
node pointing
to DSI-master, driver will find the device in runtime and use ops as needed
(additional ops/helpers required).
This is also the way to support devices which can be controlled by DSI
and I2C
in the same time. Anyway I suspect such scenario will be quite rare.

>
> And, if we want to separate the video and control, I see no reason to
> explicitly require the video side to be present. I.e. we could as well
> have a DSI peripheral that has only the control bus used. How would that
> reflect to, say, the DT presentation? Say, if we have a version A of the
> encoder, we could have DT data like this (just a rough example):
>
> soc-dsi {
> 	encoder {
> 		input: endpoint {
> 			remote-endpoint = <&soc-dsi-ep>;
Here I would replace &soc-dsi-ep by phandle to display controller/crtc/....

> 			/* configuration for the DSI lanes */
> 			dsi-lanes = <0 1 2 3 4 5>;
Wow, quite advanced DSI.
> 		};
> 	};
> };
>
> So the encoder would be places inside the SoC's DSI node, similar to how
> an i2c device would be placed inside SoC's i2c node. DSI configuration
> would be inside the video endpoint data.
>
> Version B would be almost the same:
>
> &i2c0 {
> 	encoder {
> 		input: endpoint {
> 			remote-endpoint = <&soc-dsi-ep>;
&soc-dsi-ep => &disp-ctrl-ep
> 			/* configuration for the DSI lanes */
> 			dsi-lanes = <0 1 2 3 4 5>;
> 		};
> 	};
> };
>
> Now, how would the video-bus-less device be defined?
> It'd be inside the
> soc-dsi node, that's clear. Where would the DSI lane configuration be?
> Not inside 'endpoint' node, as that's for video and wouldn't exist in
> this case. Would we have the same lane configuration in two places, once
> for video and once for control?
I think it is control setting, so it should be put outside endpoint node.
Probably it could be placed in encoder node.
>
> I agree that having DSI/DBI control and video separated would be
> elegant. But I'd like to hear what is the technical benefit of that? At
> least to me it's clearly more complex to separate them than to keep them
> together (to the extent that I don't yet see how it is even possible),
> so there must be a good reason for the separation. I don't understand
> that reason. What is it?
Roughly speaking it is a question where is the more convenient place to
put bunch
of opses, technically both solutions can be somehow implemented.

Pros of mipi bus:
- no fake entity in CDF, with fake opses, I have to use similar entities
in MIPI-CSI
camera pipelines and it complicates life without any benefit(at least
from user side),
- CDF models only video buses, control bus is a domain of Linux buses,
- less platform_bus abusing,
- better device tree topology (at least for common cases),
- quite simple in case of typical devices.

Regards
Andrzej
>
>  Tomi
>
>


^ permalink raw reply

* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Tomi Valkeinen @ 2013-10-11 12:30 UTC (permalink / raw)
  To: Andrzej Hajda, Laurent Pinchart
  Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
	Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
	Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
	Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <5257DEB5.6000708@samsung.com>

[-- Attachment #1: Type: text/plain, Size: 7199 bytes --]

On 11/10/13 14:19, Andrzej Hajda wrote:
> On 10/11/2013 08:37 AM, Tomi Valkeinen wrote:

>> The minimum bta-timeout should be deducable from the DSI bus speed,
>> shouldn't it? Thus there's no need to define it anywhere.
> Hmm, specification says "This specified period shall be longer then
> the maximum possible turnaround delay for the unit to which the
> turnaround request was sent".

Ah, you're right. We can't know how long the peripheral will take
responding. I was thinking of something that only depends on the
bus-speed and the timings for that.

> If I undrestand correctly you think about CDF topology like below:
> 
> DispContr(SoC) ---> DSI-master(SoC) ---> encoder(DSI or I2C)
> 
> But I think with mipi-dsi-bus topology could look like:
> 
> DispContr(SoC) ---> encoder(DSI or I2C)
> 
> DSI-master will not have its own entity, in the graph it could be
> represented
> by the link(--->), as it really does not process the video, only
> transports it.

At least in OMAP, the SoC's DSI-master receives parallel RGB data from
DISPC, and encodes it to DSI. Isn't that processing? It's basically a
DPI-to-DSI encoder. And it's not a simple pass-through, the DSI video
timings could be considerably different than the DPI timings.

> In case of version A I think everything is clear.
> In case of version B it does not seems so nice at the first sight, but
> still seems quite straightforward to me - special plink in encoder's
> node pointing
> to DSI-master, driver will find the device in runtime and use ops as needed
> (additional ops/helpers required).
> This is also the way to support devices which can be controlled by DSI
> and I2C
> in the same time. Anyway I suspect such scenario will be quite rare.

Okay, so if I gather it right, you say there would be something like
'dsi_adapter' (like i2c_adapter), which represents the dsi-master. And a
driver could get pointer to this, regardless of whether it the linux
device is a DSI device.

At least one issue with this approach is the endpoint problem (see below).

>> And, if we want to separate the video and control, I see no reason to
>> explicitly require the video side to be present. I.e. we could as well
>> have a DSI peripheral that has only the control bus used. How would that
>> reflect to, say, the DT presentation? Say, if we have a version A of the
>> encoder, we could have DT data like this (just a rough example):
>>
>> soc-dsi {
>> 	encoder {
>> 		input: endpoint {
>> 			remote-endpoint = <&soc-dsi-ep>;
> Here I would replace &soc-dsi-ep by phandle to display controller/crtc/....
> 
>> 			/* configuration for the DSI lanes */
>> 			dsi-lanes = <0 1 2 3 4 5>;
> Wow, quite advanced DSI.

Wha? That just means there is one clock lane and two datalanes, nothing
more =). We can select the polarity of a lane, so we describe both the
positive and negative lines there. So it says clk- is connected to pin
0, clk+ connected to pin 1, etc.

>> 		};
>> 	};
>> };
>>
>> So the encoder would be places inside the SoC's DSI node, similar to how
>> an i2c device would be placed inside SoC's i2c node. DSI configuration
>> would be inside the video endpoint data.
>>
>> Version B would be almost the same:
>>
>> &i2c0 {
>> 	encoder {
>> 		input: endpoint {
>> 			remote-endpoint = <&soc-dsi-ep>;
> &soc-dsi-ep => &disp-ctrl-ep
>> 			/* configuration for the DSI lanes */
>> 			dsi-lanes = <0 1 2 3 4 5>;
>> 		};
>> 	};
>> };
>>
>> Now, how would the video-bus-less device be defined?
>> It'd be inside the
>> soc-dsi node, that's clear. Where would the DSI lane configuration be?
>> Not inside 'endpoint' node, as that's for video and wouldn't exist in
>> this case. Would we have the same lane configuration in two places, once
>> for video and once for control?
> I think it is control setting, so it should be put outside endpoint node.
> Probably it could be placed in encoder node.

Well, one point of the endpoints is also to allow "switching" of video
devices.

For example, I could have a board with a SoC's DSI output, connected to
two DSI panels. There would be some kind of mux between, so that I can
select which of the panels is actually connected to the SoC.

Here the first panel could use 2 datalanes, the second one 4. Thus, the
DSI master would have two endpoints, the other one using 2 and the other
4 datalanes.

If we decide that kind of support is not needed, well, is there even
need for the V4L2 endpoints in the DT data at all?

>> I agree that having DSI/DBI control and video separated would be
>> elegant. But I'd like to hear what is the technical benefit of that? At
>> least to me it's clearly more complex to separate them than to keep them
>> together (to the extent that I don't yet see how it is even possible),
>> so there must be a good reason for the separation. I don't understand
>> that reason. What is it?
> Roughly speaking it is a question where is the more convenient place to
> put bunch
> of opses, technically both solutions can be somehow implemented.

Well, it's also about dividing a single physical bus into two separate
interfaces to it. It sounds to me that it would be much more complex
with locking. With a single API, we can just say "the caller handles
locking". With two separate interfaces, there must be locking at the
lower level.

> Pros of mipi bus:
> - no fake entity in CDF, with fake opses, I have to use similar entities
> in MIPI-CSI
> camera pipelines and it complicates life without any benefit(at least
> from user side),

You mean the DSI-master? I don't see how it's "fake", it's a video
processing unit that has to be configured. Even if we forget the control
side, and just think about plain video stream with DSI video mode,
there's are things to configure with it.

What kind of issues you have in the CSI side, then?

> - CDF models only video buses, control bus is a domain of Linux buses,

Yes, but in this case the buses are the same. It makes me a bit nervous
to have two separate ways (video and control) to use the same bus, in a
case like video where timing is critical.

So yes, we can consider video and control buses as "virtual" buses, and
the actual transport is the DSI bus. Maybe it can be done. It just makes
me a bit nervous =).

> - less platform_bus abusing,

Well, platform.txt says

"This pseudo-bus
is used to connect devices on busses with minimal infrastructure,
like those used to integrate peripherals on many system-on-chip
processors, or some "legacy" PC interconnects; as opposed to large
formally specified ones like PCI or USB."

I don't think DSI and DBI as platform bus is that far from the
description. They are "simple", no probing point-to-point (in practice)
buses. There's not much "bus" to speak of, just a point-to-point link.

> - better device tree topology (at least for common cases),

Even if we use platform devices for DSI peripherals, we can have them
described under the DSI master node.

> - quite simple in case of typical devices.

Still more complex than single API for both video and control =).

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Andrzej Hajda @ 2013-10-11 14:16 UTC (permalink / raw)
  To: Tomi Valkeinen, Laurent Pinchart
  Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
	Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
	Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
	Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <5257EF6A.4020005@ti.com>

On 10/11/2013 02:30 PM, Tomi Valkeinen wrote:
> On 11/10/13 14:19, Andrzej Hajda wrote:
>> On 10/11/2013 08:37 AM, Tomi Valkeinen wrote:
>>> The minimum bta-timeout should be deducable from the DSI bus speed,
>>> shouldn't it? Thus there's no need to define it anywhere.
>> Hmm, specification says "This specified period shall be longer then
>> the maximum possible turnaround delay for the unit to which the
>> turnaround request was sent".
> Ah, you're right. We can't know how long the peripheral will take
> responding. I was thinking of something that only depends on the
> bus-speed and the timings for that.
>
>> If I undrestand correctly you think about CDF topology like below:
>>
>> DispContr(SoC) ---> DSI-master(SoC) ---> encoder(DSI or I2C)
>>
>> But I think with mipi-dsi-bus topology could look like:
>>
>> DispContr(SoC) ---> encoder(DSI or I2C)
>>
>> DSI-master will not have its own entity, in the graph it could be
>> represented
>> by the link(--->), as it really does not process the video, only
>> transports it.
> At least in OMAP, the SoC's DSI-master receives parallel RGB data from
> DISPC, and encodes it to DSI. Isn't that processing? It's basically a
> DPI-to-DSI encoder. And it's not a simple pass-through, the DSI video
> timings could be considerably different than the DPI timings.
Picture size, content and format is the same on input and on output of DSI.
The same bits which enters DSI appears on the output. Internally bits
order can
be different but practically you are configuring DSI master and slave
with the same format.

If you create DSI entity you will have to always set the same format and
size on DSI input, DSI output and encoder input.
If you skip creating DSI entity you loose nothing, and you do not need
to take care of it.

>
>> In case of version A I think everything is clear.
>> In case of version B it does not seems so nice at the first sight, but
>> still seems quite straightforward to me - special plink in encoder's
>> node pointing
>> to DSI-master, driver will find the device in runtime and use ops as needed
>> (additional ops/helpers required).
>> This is also the way to support devices which can be controlled by DSI
>> and I2C
>> in the same time. Anyway I suspect such scenario will be quite rare.
> Okay, so if I gather it right, you say there would be something like
> 'dsi_adapter' (like i2c_adapter), which represents the dsi-master. And a
> driver could get pointer to this, regardless of whether it the linux
> device is a DSI device.
>
> At least one issue with this approach is the endpoint problem (see below).
>
>>> And, if we want to separate the video and control, I see no reason to
>>> explicitly require the video side to be present. I.e. we could as well
>>> have a DSI peripheral that has only the control bus used. How would that
>>> reflect to, say, the DT presentation? Say, if we have a version A of the
>>> encoder, we could have DT data like this (just a rough example):
>>>
>>> soc-dsi {
>>> 	encoder {
>>> 		input: endpoint {
>>> 			remote-endpoint = <&soc-dsi-ep>;
>> Here I would replace &soc-dsi-ep by phandle to display controller/crtc/....
>>
>>> 			/* configuration for the DSI lanes */
>>> 			dsi-lanes = <0 1 2 3 4 5>;
>> Wow, quite advanced DSI.
> Wha? That just means there is one clock lane and two datalanes, nothing
> more =). We can select the polarity of a lane, so we describe both the
> positive and negative lines there. So it says clk- is connected to pin
> 0, clk+ connected to pin 1, etc.
OK in V4L binding world it means DSI with six lanes :)
>
>>> 		};
>>> 	};
>>> };
>>>
>>> So the encoder would be places inside the SoC's DSI node, similar to how
>>> an i2c device would be placed inside SoC's i2c node. DSI configuration
>>> would be inside the video endpoint data.
>>>
>>> Version B would be almost the same:
>>>
>>> &i2c0 {
>>> 	encoder {
>>> 		input: endpoint {
>>> 			remote-endpoint = <&soc-dsi-ep>;
>> &soc-dsi-ep => &disp-ctrl-ep
>>> 			/* configuration for the DSI lanes */
>>> 			dsi-lanes = <0 1 2 3 4 5>;
>>> 		};
>>> 	};
>>> };
>>>
>>> Now, how would the video-bus-less device be defined?
>>> It'd be inside the
>>> soc-dsi node, that's clear. Where would the DSI lane configuration be?
>>> Not inside 'endpoint' node, as that's for video and wouldn't exist in
>>> this case. Would we have the same lane configuration in two places, once
>>> for video and once for control?
>> I think it is control setting, so it should be put outside endpoint node.
>> Probably it could be placed in encoder node.
> Well, one point of the endpoints is also to allow "switching" of video
> devices.
>
> For example, I could have a board with a SoC's DSI output, connected to
> two DSI panels. There would be some kind of mux between, so that I can
> select which of the panels is actually connected to the SoC.
>
> Here the first panel could use 2 datalanes, the second one 4. Thus, the
> DSI master would have two endpoints, the other one using 2 and the other
> 4 datalanes.
>
> If we decide that kind of support is not needed, well, is there even
> need for the V4L2 endpoints in the DT data at all?
Hmm, both panels connected to one endpoint of dispc ?
The problem I see is which driver should handle panel switching,
but this is question about hardware design as well. If this is realized
by dispc I have told already the solution. If this is realized by other
device I do not see a problem to create corresponding CDF entity,
or maybe it can be handled by "Pipeline Controller" ???
>
>>> I agree that having DSI/DBI control and video separated would be
>>> elegant. But I'd like to hear what is the technical benefit of that? At
>>> least to me it's clearly more complex to separate them than to keep them
>>> together (to the extent that I don't yet see how it is even possible),
>>> so there must be a good reason for the separation. I don't understand
>>> that reason. What is it?
>> Roughly speaking it is a question where is the more convenient place to
>> put bunch
>> of opses, technically both solutions can be somehow implemented.
> Well, it's also about dividing a single physical bus into two separate
> interfaces to it. It sounds to me that it would be much more complex
> with locking. With a single API, we can just say "the caller handles
> locking". With two separate interfaces, there must be locking at the
> lower level.
We say then: callee handles locking :)
>
>> Pros of mipi bus:
>> - no fake entity in CDF, with fake opses, I have to use similar entities
>> in MIPI-CSI
>> camera pipelines and it complicates life without any benefit(at least
>> from user side),
> You mean the DSI-master? I don't see how it's "fake", it's a video
> processing unit that has to be configured. Even if we forget the control
> side, and just think about plain video stream with DSI video mode,
> there's are things to configure with it.
>
> What kind of issues you have in the CSI side, then?
Not real issues, just needless calls to configure CSI entity pads,
with the same format and picture sizes as in camera.

>
>> - CDF models only video buses, control bus is a domain of Linux buses,
> Yes, but in this case the buses are the same. It makes me a bit nervous
> to have two separate ways (video and control) to use the same bus, in a
> case like video where timing is critical.
>
> So yes, we can consider video and control buses as "virtual" buses, and
> the actual transport is the DSI bus. Maybe it can be done. It just makes
> me a bit nervous =).
>
>> - less platform_bus abusing,
> Well, platform.txt says
>
> "This pseudo-bus
> is used to connect devices on busses with minimal infrastructure,
> like those used to integrate peripherals on many system-on-chip
> processors, or some "legacy" PC interconnects; as opposed to large
> formally specified ones like PCI or USB."
>
> I don't think DSI and DBI as platform bus is that far from the
> description. They are "simple", no probing point-to-point (in practice)
> buses. There's not much "bus" to speak of, just a point-to-point link.
Next section:

Platform devices
~~~~~~~~~~~~~~~~
Platform devices are devices that typically appear as autonomous
entities in the system. This includes legacy port-based devices and
host bridges to peripheral buses, and most controllers integrated
into system-on-chip platforms.  What they usually have in common
is direct addressing from a CPU bus.  Rarely, a platform_device will
be connected through a segment of some other kind of bus; but its
registers will still be directly addressable.


>> - better device tree topology (at least for common cases),
> Even if we use platform devices for DSI peripherals, we can have them
> described under the DSI master node.
Sorry, I meant rather Linux device tree topology, not DT.
>
>> - quite simple in case of typical devices.
> Still more complex than single API for both video and control =).
I agree.

Andrzej

>  Tomi
>
>


^ permalink raw reply

* Re: [PATCH/RFC v3 00/19] Common Display Framework
From: Tomi Valkeinen @ 2013-10-11 14:45 UTC (permalink / raw)
  To: Andrzej Hajda, Laurent Pinchart
  Cc: linux-fbdev, dri-devel, Jesse Barnes, Benjamin Gaignard, Tom Gall,
	Kyungmin Park, linux-media, Stephen Warren, Mark Zhang,
	Alexandre Courbot, Ragesh Radhakrishnan, Thomas Petazzoni,
	Sunil Joshi, Maxime Ripard, Vikas Sajjan, Marcus Lorentzon
In-Reply-To: <5258084A.9000509@samsung.com>

[-- Attachment #1: Type: text/plain, Size: 6916 bytes --]

On 11/10/13 17:16, Andrzej Hajda wrote:

> Picture size, content and format is the same on input and on output of DSI.
> The same bits which enters DSI appears on the output. Internally bits
> order can
> be different but practically you are configuring DSI master and slave
> with the same format.
> 
> If you create DSI entity you will have to always set the same format and
> size on DSI input, DSI output and encoder input.
> If you skip creating DSI entity you loose nothing, and you do not need
> to take care of it.

Well, this is really a different question from the bus problem. But
nothing says the DSI master cannot change the format or even size. For
sure it can change the video timings. The DSI master could even take two
parallel inputs, and combine them into one DSI output. You don't can't
what all the possible pieces of hardware do =).

If you have a bigger IP block that internally contains the DISPC and the
DSI, then, yes, you can combine them into one display entity. I don't
think that's correct, though. And if the DISPC and DSI are independent
blocks, then especially I think there must be an entity for the DSI
block, which will enable the powers, clocks, etc, when needed.

>> Well, one point of the endpoints is also to allow "switching" of video
>> devices.
>>
>> For example, I could have a board with a SoC's DSI output, connected to
>> two DSI panels. There would be some kind of mux between, so that I can
>> select which of the panels is actually connected to the SoC.
>>
>> Here the first panel could use 2 datalanes, the second one 4. Thus, the
>> DSI master would have two endpoints, the other one using 2 and the other
>> 4 datalanes.
>>
>> If we decide that kind of support is not needed, well, is there even
>> need for the V4L2 endpoints in the DT data at all?
> Hmm, both panels connected to one endpoint of dispc ?
> The problem I see is which driver should handle panel switching,
> but this is question about hardware design as well. If this is realized
> by dispc I have told already the solution. If this is realized by other
> device I do not see a problem to create corresponding CDF entity,
> or maybe it can be handled by "Pipeline Controller" ???

Well the switching could be automatic, when the panel power is enabled,
the DSI mux is switched for that panel. It's not relevant.

We still have two different endpoint configurations for the same
DSI-master port. If that configuration is in the DSI-master's port node,
not inside an endpoint data, then that can't be supported.

>>>> I agree that having DSI/DBI control and video separated would be
>>>> elegant. But I'd like to hear what is the technical benefit of that? At
>>>> least to me it's clearly more complex to separate them than to keep them
>>>> together (to the extent that I don't yet see how it is even possible),
>>>> so there must be a good reason for the separation. I don't understand
>>>> that reason. What is it?
>>> Roughly speaking it is a question where is the more convenient place to
>>> put bunch
>>> of opses, technically both solutions can be somehow implemented.
>> Well, it's also about dividing a single physical bus into two separate
>> interfaces to it. It sounds to me that it would be much more complex
>> with locking. With a single API, we can just say "the caller handles
>> locking". With two separate interfaces, there must be locking at the
>> lower level.
> We say then: callee handles locking :)

Sure, but my point was that the caller handling the locking is much
simpler than the callee handling locking. And the latter causes
atomicity issues, as the other API could be invoked in between two calls
for the first API.

But note that I'm not saying we should not implement bus model just
because it's more complex. We should go for bus model if it's better. I
just want to bring up these complexities, which I feel are quite more
difficult than with the simpler model.

>>> Pros of mipi bus:
>>> - no fake entity in CDF, with fake opses, I have to use similar entities
>>> in MIPI-CSI
>>> camera pipelines and it complicates life without any benefit(at least
>>> from user side),
>> You mean the DSI-master? I don't see how it's "fake", it's a video
>> processing unit that has to be configured. Even if we forget the control
>> side, and just think about plain video stream with DSI video mode,
>> there's are things to configure with it.
>>
>> What kind of issues you have in the CSI side, then?
> Not real issues, just needless calls to configure CSI entity pads,
> with the same format and picture sizes as in camera.

Well, the output of a component A is surely the same as the input of
component B, if B receives the data from A. So that does sound useless.
I don't do that kind of calls in my model.

>>> - CDF models only video buses, control bus is a domain of Linux buses,
>> Yes, but in this case the buses are the same. It makes me a bit nervous
>> to have two separate ways (video and control) to use the same bus, in a
>> case like video where timing is critical.
>>
>> So yes, we can consider video and control buses as "virtual" buses, and
>> the actual transport is the DSI bus. Maybe it can be done. It just makes
>> me a bit nervous =).
>>
>>> - less platform_bus abusing,
>> Well, platform.txt says
>>
>> "This pseudo-bus
>> is used to connect devices on busses with minimal infrastructure,
>> like those used to integrate peripherals on many system-on-chip
>> processors, or some "legacy" PC interconnects; as opposed to large
>> formally specified ones like PCI or USB."
>>
>> I don't think DSI and DBI as platform bus is that far from the
>> description. They are "simple", no probing point-to-point (in practice)
>> buses. There's not much "bus" to speak of, just a point-to-point link.
> Next section:
> 
> Platform devices
> ~~~~~~~~~~~~~~~~
> Platform devices are devices that typically appear as autonomous
> entities in the system. This includes legacy port-based devices and
> host bridges to peripheral buses, and most controllers integrated
> into system-on-chip platforms.  What they usually have in common
> is direct addressing from a CPU bus.  Rarely, a platform_device will
> be connected through a segment of some other kind of bus; but its
> registers will still be directly addressable.

Yep, "typically" and "rarely" =). I agree, it's not clear. I think there
are things with DBI/DSI that clearly point to a platform device, but
also the other way.

>>> - better device tree topology (at least for common cases),
>> Even if we use platform devices for DSI peripherals, we can have them
>> described under the DSI master node.
> Sorry, I meant rather Linux device tree topology, not DT.

We can have the DSI peripheral platform devices as children of the
DSI-master device.

 Tomi



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 901 bytes --]

^ permalink raw reply

* [PATCH] do_register_framebuffer() fix potential deadlock
From: Alexei Starovoitov @ 2013-10-11 22:00 UTC (permalink / raw)
  To: linux-fbdev

commit 50e244cc79
"fb: rework locking to fix lock ordering on takeover"

fixed the locking, but in one place the console_lock() and lock_fb_info()
seem to be in the wrong order vs the rest

to reproduce just boot and wait till blank

===========================
[ INFO: possible circular locking dependency detected ]
3.12.0-rc3+ #50 Not tainted
-------------------------------------------------------
kworker/0:0/4 is trying to acquire lock:
 (&fb_info->lock){+.+.+.}, at: [<ffffffff813ec786>] lock_fb_info+0x26/0x60

but task is already holding lock:
 (console_lock){+.+.+.}, at: [<ffffffff8146acf3>] console_callback+0x13/0x140

which lock already depends on the new lock.

the existing dependency chain (in reverse order) is:

-> #1 (console_lock){+.+.+.}:
       [<ffffffff810cf002>] lock_acquire+0x92/0x1d0
       [<ffffffff810b3467>] console_lock+0x77/0x80
       [<ffffffff813edc14>] register_framebuffer+0x1d4/0x320
       [<ffffffff81403e4a>] vesafb_probe+0x5ba/0xa70
       [<ffffffff814b7393>] platform_drv_probe+0x43/0x80
       [<ffffffff814b52cb>] driver_probe_device+0x7b/0x220
       [<ffffffff814b551b>] __driver_attach+0xab/0xb0
       [<ffffffff814b32bd>] bus_for_each_dev+0x5d/0xa0
       [<ffffffff814b4cde>] driver_attach+0x1e/0x20
       [<ffffffff814b486f>] bus_add_driver+0x10f/0x2a0
       [<ffffffff814b5be4>] driver_register+0x64/0xf0
       [<ffffffff814b6b8a>] __platform_driver_register+0x4a/0x50
       [<ffffffff81d5e863>] vesafb_driver_init+0x12/0x14
       [<ffffffff8100031a>] do_one_initcall+0x11a/0x170
       [<ffffffff81d21fed>] kernel_init_freeable+0x13e/0x1cd
       [<ffffffff8174a49e>] kernel_init+0xe/0xf0
       [<ffffffff8176c86c>] ret_from_fork+0x7c/0xb0

-> #0 (&fb_info->lock){+.+.+.}:
       [<ffffffff810ce883>] __lock_acquire+0x1c63/0x1d50
       [<ffffffff810cf002>] lock_acquire+0x92/0x1d0
       [<ffffffff8175dc94>] mutex_lock_nested+0x74/0x380
       [<ffffffff813ec786>] lock_fb_info+0x26/0x60
       [<ffffffff813fb08b>] fbcon_blank+0x29b/0x2e0
       [<ffffffff81468898>] do_blank_screen+0x1d8/0x280
       [<ffffffff8146ad44>] console_callback+0x64/0x140
       [<ffffffff81074b48>] process_one_work+0x1d8/0x6a0
       [<ffffffff810754bb>] worker_thread+0x11b/0x370
       [<ffffffff8107e36a>] kthread+0xea/0xf0
       [<ffffffff8176c86c>] ret_from_fork+0x7c/0xb0

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(console_lock);
                               lock(&fb_info->lock);
                               lock(console_lock);
  lock(&fb_info->lock);

 *** DEADLOCK ***

Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
 drivers/video/fbmem.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/video/fbmem.c b/drivers/video/fbmem.c
index dacaf74..f12ab1c 100644
--- a/drivers/video/fbmem.c
+++ b/drivers/video/fbmem.c
@@ -1660,12 +1660,14 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	registered_fb[i] = fb_info;
 
 	event.info = fb_info;
-	if (!lock_fb_info(fb_info))
-		return -ENODEV;
 	console_lock();
+	if (!lock_fb_info(fb_info)) {
+		console_unlock();
+		return -ENODEV;
+	}
 	fb_notifier_call_chain(FB_EVENT_FB_REGISTERED, &event);
-	console_unlock();
 	unlock_fb_info(fb_info);
+	console_unlock();
 	return 0;
 }
 
-- 
1.7.9.5


^ permalink raw reply related

* [PATCH] fbdev: fix error return code in metronomefb_probe()
From: Wei Yongjun @ 2013-10-12  6:25 UTC (permalink / raw)
  To: linux-fbdev

From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>

Fix to return a negative error code from the error handling
case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
 drivers/video/metronomefb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/metronomefb.c b/drivers/video/metronomefb.c
index f30150d..0b36160 100644
--- a/drivers/video/metronomefb.c
+++ b/drivers/video/metronomefb.c
@@ -690,7 +690,8 @@ static int metronomefb_probe(struct platform_device *dev)
 		goto err_csum_table;
 	}
 
-	if (board->setup_irq(info))
+	retval = board->setup_irq(info);
+	if (retval)
 		goto err_csum_table;
 
 	retval = metronome_init_regs(par);


^ permalink raw reply related

* Re: [PATCH 2/2] framebuffer: Remove pmag-aa-fb
From: Maciej W. Rozycki @ 2013-10-12 13:08 UTC (permalink / raw)
  To: Joe Perches
  Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Linux Fbdev development list,
	linux-kernel@vger.kernel.org, Linux MIPS Mailing List
In-Reply-To: <alpine.LFD.2.03.1309201946430.8379@linux-mips.org>

Joe,

 Just a quick update.

On Sun, 22 Sep 2013, Maciej W. Rozycki wrote:

>  As to the PMAG-AA board itself -- well, this is indeed a very rare item, 
> but I happen to have a specimen.  To support it properly I'll first have 
> to wire it to a monitor somehow though; signalling is standard, 1.0 Vpp 
> composite monochrome, but what looks to me like a type F connector is used 
> for video output, quite unusually for a graphics card (and for DEC itself 
> too as 3W3 was their usual video socket).  It looks to me like converting 
> it to BNC and then a standard DE-15 VGA connector (via the green line) 
> will be the easiest way to get image produced by the adapter on a 
> contemporary monitor (sync-on-green required of course, but with LCD 
> devices being the norm now that seems less of a problem these days).

 So more weirdly even that's actually a TNC connector rather than a type F 
one.  I've got a suitable TNC->BNC adapter now (although regrettably such 
adapters seem to be available as 50Ù parts only; hopefully any distortion 
won't be too significant or maybe my digital monitor will even be able to 
compensate it, but at £1.76 (~$2.64) per item it's certainly worth trying 
before resorting to the original DEC TNC->BNC cable still apparently 
available from second-hand part suppliers at ~£80/$120 per a mere 1ft 
part) and a BNC->DE-15 cable is on the way.

> > --- a/drivers/video/pmag-aa-fb.c
> > +++ b/drivers/video/pmag-aa-fb.c
> > @@ -459,7 +459,7 @@ static int __init init_one(int slot)
> >  		return -EINVAL;
> >  
> >  	printk(KERN_INFO "fb%d: %s frame buffer in TC slot %d\n",
> > -	       GET_FB_IDX(ip->info.node), ip->info.modename, slot);
> > +	       ip->info.node, ip->info.modename, slot);
> >  
> >  	return 0;
> >  }
> 
>  Thanks, but the changes required are actually much more than that -- the 
> driver has never been converted to the modern TURBOchannel API.  I have 
> now dug out an old patch I was working on back in 2006 to convert this 
> driver as well as drivers/video/maxinefb.c.  I'll try to complete the two 
> drivers as soon as possible (unfortunately I can't test the latter at all; 
> it's for an onboard graphics adapter of another DECstation model), 
> although I now remember the main reason I didn't complete them back then 
> was they used an old internal API that was removed and no suitable 
> replacement provided.  I need to investigate again what that actually was 
> though (hw cursor probably).

 So I think I've got all the basic stuff covered now, including a change 
similar to your proposal as well as a conversion to the driver model/new 
TURBOchannel support infrastructure.  But what I remembered is actually 
right, the issue is wiring hardware cursor support into fbcon.  The driver 
uses its own display_switch structure with its own aafbcon_cursor handler 
to use the twin onboard Bt431 chips for cursor generation (there's also 
aafbcon_set_font that pokes at the Bt431s for cursor dimension changes).  
I need to figure out what the best way will be to make the fbcon subsystem 
support such an arrangement and that'll take me a little bit yet, so 
please be patient.

 Note that the board is weird enough to have a 1-bit (true monochrome) 
graphics plane, however the Bt455 used by the MX graphics adapter for 
screen image generation is a 4-bit grey-scale video RAMDAC (only the LSB 
inputs of its pixel port are wired to the graphics plane) and the twin 
Bt431s use the overlay plane to produce a 2-bit grey-scale cursor.  So we 
do want to use the hardware cursor to be able to make it prominent among 
the characters displayed throughout the screen and a software-generated 
cursor cannot really substitute what hardware provides.

  Maciej

^ permalink raw reply

* Re: [PATCH 2/2] framebuffer: Remove pmag-aa-fb
From: Joe Perches @ 2013-10-12 16:08 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: Geert Uytterhoeven, Jean-Christophe Plagniol-Villard,
	Tomi Valkeinen, Linux Fbdev development list,
	linux-kernel@vger.kernel.org, Linux MIPS Mailing List
In-Reply-To: <alpine.LFD.2.03.1310121308310.10951@linux-mips.org>

On Sat, 2013-10-12 at 14:08 +0100, Maciej W. Rozycki wrote:
[]
> So I think I've got all the basic stuff covered now, including a change 
> similar to your proposal as well as a conversion to the driver model/new 
> TURBOchannel support infrastructure.  But what I remembered is actually 
> right, the issue is wiring hardware cursor support into fbcon.  The driver 
> uses its own display_switch structure with its own aafbcon_cursor handler 
> to use the twin onboard Bt431 chips for cursor generation (there's also 
> aafbcon_set_font that pokes at the Bt431s for cursor dimension changes).  
> I need to figure out what the best way will be to make the fbcon subsystem 
> support such an arrangement and that'll take me a little bit yet, so 
> please be patient.
> 
> Note that the board is weird enough to have a 1-bit (true monochrome) 
> graphics plane, however the Bt455 used by the MX graphics adapter for 
> screen image generation is a 4-bit grey-scale video RAMDAC (only the LSB 
> inputs of its pixel port are wired to the graphics plane) and the twin 
> Bt431s use the overlay plane to produce a 2-bit grey-scale cursor.  So we 
> do want to use the hardware cursor to be able to make it prominent among 
> the characters displayed throughout the screen and a software-generated 
> cursor cannot really substitute what hardware provides.

I hope you're enjoying tinkering with old toys.

Best of luck getting it going.



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox