Re: [PATCH/RFC v3 00/19] Common Display Framework

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

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

On 17/10/13 15:26, Andrzej Hajda wrote:

> I am not sure what exactly the encoder performs, if this is only image
> transport from dispc to panel CDF pipeline in both cases should look like:
> dispc ----> panel
> The only difference is that panels will be connected via different Linux bus
> adapters, but it will be irrelevant to CDF itself. In this case I would say
> this is DSI-master rather than encoder, or at least that the only
> function of the
> encoder is DSI.

Yes, as I said, it's up to the driver writer how he wants to use CDF. If
he doesn't see the point of representing the SoC's DSI encoder as a
separate CDF entity, nobody forces him to do that.

On OMAP, we have single DISPC with multiple parallel outputs, and a
bunch of encoder IPs (MIPI DPI, DSI, DBI, etc). Each encoder IP can be
connected to some of the DISPC's output. In this case, even if the DSI
encoder does nothing special, I see it much better to represent the DSI
encoder as a CDF entity so that the links between DISPC, DSI, and the
DSI peripherals are all there.

> If display_timings on input and output differs, I suppose it should be
> modeled
> as display_entity, as this is an additional functionality(not covered by
> DSI standard AFAIK).

Well, DSI standard is about the DSI output. Not about the encoder's
input, or the internal operation of the encoder.

>>> Of course there are some settings which are not panel dependent and those
>>> should reside in DSI node.
>> Exactly. And when the two panels require different non-panel-dependent
>> settings, how do you represent them in the DT data?
> 
> non-panel-dependent setting cannot depend on panel, by definition :)

With "non-panel-dependent" setting I meant something that is a property
of the DSI master device, but still needs to be configured differently
for each panel.

Say, pin configuration. When using panel A, the first pin of the DSI
block could be clock+. With panel B, the first pin could be clock-. This
configuration is about DSI master, but it is different for each panel.

If we have separate endpoint in the DSI master for each panel, this data
can be there. If we don't have the endpoint, as is the case with
separate control bus, where is that data?

>>> Could you describe such scenario?
>> If we have two independent APIs, ctrl and video, that affect the same
>> underlying hardware, the DSI bus, we could have a scenario like this:
>>
>> thread 1:
>>
>> ctrl->op_foo();
>> ctrl->op_bar();
>>
>> thread 2:
>>
>> video->op_baz();
>>
>> Even if all those ops do locking properly internally, the fact that
>> op_baz() can be called in between op_foo() and op_bar() may cause problems.
>>
>> To avoid that issue with two APIs we'd need something like:
>>
>> thread 1:
>>
>> ctrl->lock();
>> ctrl->op_foo();
>> ctrl->op_bar();
>> ctrl->unlock();
>>
>> thread 2:
>>
>> video->lock();
>> video->op_baz();
>> video->unlock();
> I should mention I was asking for real hw/drivers configuration.
> I do not know what do you mean with video->op_baz() ?
> DSI-master is not modeled in CDF, and only CDF provides video
> operations.

It was just an example of the additional complexity with regarding
locking when using two APIs.

The point is that if the panel driver has two pointers (i.e. API), one
for the control bus, one for the video bus, and ops on both buses affect
the same hardware, the locking is not easy.

If, on the other hand, the panel driver only has one API to use, it's
simple to require the caller to handle any locking.

> I guess one scenario, when two panels are connected to single DSI-master.
> In such case both can call DSI ops, but I do not know how do you want to
> prevent it in case of your CDF-T implementation.

No, that was not the case I was describing. This was about a single panel.

If we have two independent APIs, we need to define how locking is
managed for those APIs. Even if in practice both APIs are used by the
same driver, and the driver can manage the locking, that's not really a
valid requirement. It'd be almost the same as requiring that gpio API
cannot be called at the same time as i2c API.

 Tomi



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