On 11/25/2025 6:06 PM, Heikki Krogerus wrote: > Tue, Nov 25, 2025 at 10:23:02AM +0800, Chaoyi Chen kirjoitti: >> On 11/25/2025 12:33 AM, Greg Kroah-Hartman wrote: >>> On Mon, Nov 24, 2025 at 04:05:53PM +0800, Chaoyi Chen wrote: >>>> Hi Greg, >>>> >>>> On 11/24/2025 3:10 PM, Greg Kroah-Hartman wrote: >>>> >>>>> On Mon, Nov 24, 2025 at 09:40:03AM +0800, Chaoyi Chen wrote: >>>>>> Hi Greg, >>>>>> >>>>>> On 11/21/2025 10:07 PM, Greg Kroah-Hartman wrote: >>>>>>> On Thu, Nov 20, 2025 at 10:23:33AM +0800, Chaoyi Chen wrote: >>>>>>>> From: Chaoyi Chen <[email protected]> >>>>>>>> >>>>>>>> Some other part of kernel may want to know the event of typec bus. >>>>>>> Be specific, WHAT part of the kernel will need to know this? >>>>>> For now, it is DRM. >>>>> Then say this. >>>> Okay, please refer to the discussion below. >>>> >>>>>>> And why a new notifier, why not just use the existing notifiers that you >>>>>>> already have? And what is this going to be used for? >>>>>> We have discussed this before, but the current bus notifier cannot >>>>>> achieve the expected notification [0]. >>>>>> >>>>>> [0] https://lore.kernel.org/all/[email protected]/ >>>>> Then you need to document the heck out of this in the changelog text. >>>>> But I'm still not quite understanding why the bus notifier does not work >>>>> here, as you only want this information if the usb device is bound to >>>>> the bus there, you do not want to know this if it did not complete. >>>>> >>>>> That thread says you want this not "too late", but why? What is the >>>>> problem there, and how will you handle your code getting loaded after >>>>> the typec code is loaded? Notifier callbacks don't work for that >>>>> situation, right? >>>> In fact, the typec_register_altmode() function generates two >>>> registered events. The first one is the registered event of the port >>>> device, and the second one is the registered event of the partner >>>> device. The second one event only occurs after a Type-C device is >>>> inserted. >>>> The bus notifier event does not actually take effect for the port >>>> device, because it only sets the bus for the partner device: >>>> >>>> /* The partners are bind to drivers */ >>>> if (is_typec_partner(parent)) >>>> alt->adev.dev.bus = &typec_bus; >>> Setting the bus is correct, then it needs to be registered with the >>> driver core so the bus link shows up (and a driver is bound to it.) >>> That is when the bus notifier can happen, right? >> Yes, this is valid for the partner device. But for the port device, since >> the bus is not specified here, the corresponding bus notifier will not take >> effect. > > Perhaps we should just fix this part and make also the port altmodes > part of the bus. > > Some background, in case this is not clear; the port alternate mode > devices represent the capability of the ports to support specific > alternate modes. The partner alternate mode devices do the same > for the partner devices attached to the ports, but on top of the > showing the capability, they are also used for the alternate mode > specific communication using the USB Power Delivery VDM (vendor > defined messages). That's why only the partner altmodes are bound > to the generic alternate mode drivers. > > And that's why the hack where the port altmodes are not added to the > bus. But maybe it's not needed. > > Chaoyi, can you try the attached patch? >
Thank you for providing the background information. Maybe this is what we really want. I will try this in v11 :) >>>> I hope it's not too late. In fact, the notifier here will notify DRM to >>>> establish a bridge chain. >>> What is a "bridge chain"? >> In DRM, the bridge chain is often used to describe the chain connection >> relationship >> of the output of multi level display conversion chips. The bridge chain we >> are referring to here >> is actually a chain structure formed by connecting various devices using a >> simple transparent bridge [0]. >> >> For example, the schematic diagram of a bridge chain is as follows: >> >> DP controller bridge -> DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> >> fsa4480 analog audio switch bridge -> fusb302 HPD bridge >> >> Here, apart from the DP controller bridge, the rest are transparent DRM >> bridges, which are used solely to >> describe the link relationships between various devices. >> >> >> [0] >> https://patchwork.freedesktop.org/patch/msgid/[email protected] >>> >>>> The downstream DP controller driver hopes to obtain the fwnode of the >>>> last-level Type-C device >>>> through this bridge chain to create a DRM connector. And when a device is >>>> inserted, >>>> drivers/usb/typec/altmodes/displayport.c can notify the HPD (Hot Plug >>>> Detect) event. >>> But aren't you just the driver for the "partner device"? >>> >>> If not, why isn't a real device being created that you then bind to, >>> what "fake" type of thing are you attempting to do here that would >>> require you to do this out-of-band? >> The HPD event is pass by drm_connector_oob_hotplug_event(), which does not >> use the device in Type-C. >> This function will find the corresponding DRM connector device, and the >> lookup of the DRM connector is >> done through the fwnode. >> >> And the partner device and the port device have the same fwnode. >> >>> >>>> If relying on the second event, the bridge chain may never be established, >>>> and the operations of the DP driver will be >>>> always deferred. Furthermore, other parts of the display controller driver >>>> will also be deferred accordingly. >>> What operations? What exactly is delayed? You should not be touching a >>> device before you have it on your bus, right? >> To complete the HPD operation, it is necessary to create a drm connector >> device that >> has the appropriate fwnode. This operation will be carried out by the DP >> controller driver. >> >> As you can see, since it cross multiple devices, we need to set the fwnode >> to the last device fusb302. >> This requires relying on the bridge chain. We can register bridges for >> multiple devices and then connect >> them to form a chain. The connection process is completed through >> drm_bridge_attach(). >> >> A brief example of the process of establishing a bridge chain is as follows, >> starting from the last bridge: >> >> step1: fusb302 HPD bridge >> step2: fsa4480 analog audio switch bridge -> fusb302 HPD bridge >> step3: onnn,nb7vpq904m retimer bridge -> fsa4480 analog audio switch bridge >> -> fusb302 HPD bridge >> step4: DP PHY bridge -> onnn,nb7vpq904m retimer bridge -> fsa4480 analog >> audio switch bridge -> fusb302 HPD bridge >> step5: DP controller bridge -> DP PHY bridge -> onnn,nb7vpq904m retimer >> bridge -> fsa4480 analog audio switch bridge -> fusb302 HPD bridge >> >> Step 1 is the most crucial, because essentially, regardless of whether we >> use notifiers or not, what we ultimately want to achieve is to create an HPD >> bridge. >> The DP controller needs to wait for the subsequent bridge chain to be >> established because it needs to know the connection relationships of the >> devices. >> >> The question now is when to create the HPD bridge, during the registration >> of the port device or during the registration of the partner device. >> If it's the latter, then the delay occurs here. >> >> And I don't think I'm touching the Type-C device here. I'm just using the >> bridge chain to get a suitable fwnode and create a suitable DRM connector >> device. >> The subsequent Type-C HPD events will be on the DRM connector device. >> >> This solution is somewhat complex, and I apologize once again for any >> confusion caused earlier. >> >>> >>>>>>> Notifiers are a pain, and should almost never be added. Use real >>>>>>> function calls instead. >>>>>> In v6, I used direct function calls, but had to switch to notifiers >>>>>> because couldn't resolve the dependencies between DRM and Type-C [1]. Do >>>>>> you have any good ideas? Thank you. >>>>> Only allow this DRM code to be built if typec code is enabled, do NOT >>>>> use a select, use a depends in the drm code. >>>> Sorry, I didn't get your point. Does this mean that the current notifiers >>>> approach still needs to be changed to direct function calls? >>> If you somehow convince me that the existing bus notifiers will not >>> work, yes :) >>> >>>> If so, then based on the previous discussion, typec should not depend >>>> on any DRM components. Does this mean that we should add the if >>>> (IS_REACHABLE(CONFIG_DRM_AUX_BRIDGE)) before the direct function call? >>> No, do it properly like any other function call to another subsystem. >>> >>>> Additionally, the current version of CONFIG_DRM_AUX_BRIDGE is selected >>>> by the DP driver in patch9. >>> Don't do "select" if at all possible, always try to do "depends on". >> Thank you for clarifying this. However, CONFIG_DRM_AUX_BRIDGE is not exposed >> in the menu, and it is not intended for the end user to select it by design. >> Therefore, I think there still needs to be some place to select it? > > You don't "select TYPEC", you already "depend on TYPEC", so you are > all set with this one. > > thanks, > Ah, it is. -- Best, Chaoyi
