Hi,

On 5/16/24 16:25, Maxime Ripard wrote:
On Wed, May 15, 2024 at 11:19:58PM +0800, Sui Jingfeng wrote:
Hi,


On 5/15/24 22:58, Maxime Ripard wrote:
On Wed, May 15, 2024 at 10:53:00PM +0800, Sui Jingfeng wrote:
On 5/15/24 22:30, Maxime Ripard wrote:
On Wed, May 15, 2024 at 12:53:33AM +0800, Sui Jingfeng wrote:
On 2024/5/15 00:22, Maxime Ripard wrote:
Hi,

On Tue, May 14, 2024 at 11:40:43PM +0800, Sui Jingfeng wrote:
Because a lot of implementations has already added it into their drived
class, promote it into drm_bridge core may benifits a lot. drm bridge is
a driver, it should know the underlying hardware entity.
Is there some actual benefits, or is it theoretical at this point?


I think, DRM bridge drivers could remove the 'struct device *dev'
member from their derived structure. Rely on the drm bridge core
when they need the 'struct device *' pointer.

Sure, but why do we need to do so?

The other thread you had with Jani points out that it turns out that
things are more complicated than "every bridge driver has a struct
device anyway", it creates inconsistency in the API (bridges would have
a struct device, but not other entities), and it looks like there's no
use for it anyway.

None of these things are deal-breaker by themselves, but if there's only
downsides and no upside, it's not clear to me why we should do it at all.

It can reduce boilerplate.

You're still using a conditional here.

It's for safety reason, prevent NULL pointer dereference.
drm bridge can be seen as either a software entity or a device driver.

It's fine to pass NULL if specific KMS drivers intend to see
drm bridge as a pure software entity, and for internal use only.
Both use cases are valid.

Sorry, I don't follow you. We can't NULL dereference a pointer that
doesn't exist.

Please state why we should merge this series: what does it fix or
improve, aside from the potential gain of making bridges declare one
less pointer in their private structure.

We could reduce more.

Bridge driver instances also don't have to embed 'struct i2c_client *'. We could use 'to_i2c_client(bridge->dev)' to retrieve the pointer,
where needed.

Maxime

--
Best regards
Sui

Reply via email to