Re: [PATCH v3 1/7] drm: Add DSI bus infrastructure

[Andrzej Hajda <a.hajda@samsung.com>]

On 11/18/2013 01:57 PM, Thierry Reding wrote:
> On Thu, Nov 14, 2013 at 03:15:57PM +0100, Andrzej Hajda wrote:
>> Hi Thierry,
>>
>> On 11/13/2013 10:38 PM, Thierry Reding wrote:
>>> On Tue, Nov 12, 2013 at 03:14:22PM +0100, Andrzej Hajda wrote:
>>>> Hi Thierry,
>>>>
>>>> I have already sent patch with DSI bus implementation [1].
>>>> It was posted as the first step of CDF implementation attempt,
>>>> but in fact it do not depend on CDF.
>>>>
>>>> [1]
>>>> http://www.mail-archive.com/dri-devel@lists.freedesktop.org/msg45252.html
>>> Seems like that patchset was never merged. I guess probably because it
>>> was posted as part of CDF work.
>>>
>>> Do you have any plans on continuing work on that?If not I could extract
>>> the DSI bus patch from the series, it's largely similar to the patch I
>>> proposed here, and rework it somewhat.
>> I will soon sent new patch with the current version of the bus.
>> It could be a better base to your rework.
> Any estimate on when this will be? I want to get started on this as soon
> as possible so we can get this in good shape for 3.14. I suspect that if
> I start based on the current patch, I won't have to redo everything when
> updating for the next version. Quite a bit should remain similar, right?
Patch sent.
>
>>> I'd very much like to avoid
>>> putting the code in drivers/video, though, since that's considered
>>> obsolete.
>> I have followed convention proposed by Laurent in his DBI bus.
>> It seems to me OK - DSI bus is more related to video than to drm.
>> I know that drivers/video is mostly occupied by FB drivers,
>> but according to Kconfig it is not only for FB.
>> Of course this is only my suggestion.
> I've had an IRC discussion with Dave and Rob (added on Cc), and they
> both agreed that implementing it as part of DRM would be preferred.
>
> The reason, as I've said before, is primarily that drivers/video is
> considered obsolete and new drivers and features should be implemented
> within the context of DRM/KMS. Additionally this has the benefit of
> being able to reuse the native DRM data structures and types, as well as
> helper functions, without having to go through an extra layer of
> compatibility.
>
>>>  Furthermore I think if we kept the transfer function proposed
>>> in my patch should make it easier to address Bert's comments from your
>>> posting.
>> I am not sure which part of Barts comment you are addressing.
>> Anyway I also prefer passing struct and returning ssize_t.
>> I am not sure about splitting type and channel but this seems to be a
>> minor detail.
> I find that to be a perfectly natural split. A DSI peripheral will have
> an associated virtual channel number anyway, and having a separate field
> will allow drivers to just copy that field into the dsi_msg's
> equivalent.
>
> The alternative would be to require each driver to properly encode the
> channel and data type.
Other alternatives:
- use helpers functions to call transfer op, encoding could be done by them,
- fill channel by DSI master, based on DSI slave(seems to be little
over-engineering)

But I see no big difference between them.

Regards
Andrzej
>
> Thierry