linux-kernel - Re: [PATCH] drm/arm/komeda: Remove component framework and add a simple encoder

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 12 Jul 2023 12:10:32 +0530
From:   Mohammad Faiz Abbas Rizvi <faiz.abbas@....com>
To:     Sam Ravnborg <sam@...nborg.org>
Cc:     Liviu Dudau <liviu.dudau@....com>, linux-kernel@...r.kernel.org,
        dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH] drm/arm/komeda: Remove component framework and add a
 simple encoder

Hi Sam,

On 7/4/2023 10:26 PM, Sam Ravnborg wrote:
> Hi Mohammed,
> 
> On Tue, Jul 04, 2023 at 07:19:04PM +0530, Mohammad Faiz Abbas Rizvi wrote:
>> Hi Liviu,
>>
>> On 6/29/2023 3:21 PM, Liviu Dudau wrote:
>>> Hi Faiz,
>>>
>>> Thanks for the patch and for addressing what was at some moment on my "nice to
>>> improve / cleanup" list. Sorry for the delay in responding, I had to revive
>>> the bits of an old setup to be able to test this properly, with 2 encoders
>>> attached.
>>>
>>> On Wed, Jun 21, 2023 at 02:11:16PM +0530, Faiz Abbas wrote:
>>>> The Komeda driver always expects the remote connector node to initialize
>>>> an encoder. It uses the component aggregator framework which consists
>>>> of component->bind() calls used to initialize the remote encoder and attach
>>>> it to the crtc. This is different from other drm implementations which expect
>>>> the display driver to supply a crtc and connect an encoder to it.
>>> I think both approaches are valid in DRM. We don't want to remove the component
>>> framework because it is doing the wrong thing, but because we cannot use it
>>> with drivers that implement the drm_bridge API. Given that we usually pair with
>>> a component encoder that also implements a drm_bridge, dropping support for
>>> component framework should not affect the users of the driver.
> 
> Glad to see the patch - I think this is moving things in the right
> direction.
> 
> 
> While at it do you plan to support DRM_BRIDGE_ATTACH_NO_CONNECTOR?
> 
> I did not read the patch carefully but noticed this call with no flags:
> 
>> drm_bridge_attach(&kcrtc->encoder, bridge, NULL, 0);
> 
> To add support for DRM_BRIDGE_ATTACH_NO_CONNECTOR you may need to verify
> that all relevant bridge drivers supports the flag.
> You will be told at runtime but only if the relevant bridge driver is
> used.

Just from reading the documentation on this (https://github.com/torvalds/linux/blob/master/drivers/gpu/drm/drm_bridge.c#L439) I gather this is useful for chains of multiple bridges where the connector is discovered at runtime? I'm not sure if we have a configuration like this with komeda here.

Also I'm not sure what it means for the CRTC driver to "support" this flag. Do I always call drm_bridge_attach() with it?

> 
> It can be done later and is obviously a separate patch.
> 

Sure though I cannot promise I can look into it anytime soon.

Thanks,
Faiz