On Mon, 2022-08-08 at 10:02 +0000, Lin, Wayne wrote: > [Public] > > > > > -----Original Message----- > > From: Lyude Paul <[email protected]> > > Sent: Thursday, August 4, 2022 4:28 AM > > To: Lin, Wayne <[email protected]>; [email protected]; > > [email protected]; [email protected] > > Cc: Ville Syrjälä <[email protected]>; Zuo, Jerry > > <[email protected]>; Jani Nikula <[email protected]>; Imre Deak > > <[email protected]>; Daniel Vetter <[email protected]>; Sean Paul > > <[email protected]>; Wentland, Harry <[email protected]>; Li, Sun > > peng (Leo) <[email protected]>; Siqueira, Rodrigo > > <[email protected]>; Deucher, Alexander > > <[email protected]>; Koenig, Christian > > <[email protected]>; Pan, Xinhui <[email protected]>; David > > Airlie <[email protected]>; Daniel Vetter <[email protected]>; Jani Nikula > > <[email protected]>; Joonas Lahtinen > > <[email protected]>; Rodrigo Vivi <[email protected]>; > > Tvrtko Ursulin <[email protected]>; Ben Skeggs > > <[email protected]>; Karol Herbst <[email protected]>; Kazlauskas, > > Nicholas <[email protected]>; Li, Roman > > <[email protected]>; Shih, Jude <[email protected]>; Simon Ser > > <[email protected]>; Lakha, Bhawanpreet > > <[email protected]>; Mikita Lipski <[email protected]>; > > Claudio Suarez <[email protected]>; Chen, Ian <[email protected]>; Colin Ian > > King <[email protected]>; Wu, Hersen <[email protected]>; Liu, > > Wenjing <[email protected]>; Lei, Jun <[email protected]>; Strauss, > > Michael <[email protected]>; Ma, Leo <[email protected]>; > > Thomas Zimmermann <[email protected]>; José Roberto de Souza > > <[email protected]>; He Ying <[email protected]>; Anshuman > > Gupta <[email protected]>; Andi Shyti > > <[email protected]>; Ashutosh Dixit <[email protected]>; > > Juston Li <[email protected]>; Sean Paul <[email protected]>; > > Fernando Ramos <[email protected]>; Luo Jiaxing > > <[email protected]>; Javier Martinez Canillas <[email protected]>; > > open list <[email protected]>; open list:INTEL DRM DRIVERS > > <[email protected]> > > Subject: Re: [RESEND RFC 18/18] drm/display/dp_mst: Move all payload info > > into the atomic state > > > > On Tue, 2022-07-05 at 09:10 +0000, Lin, Wayne wrote: > > > > +struct drm_dp_mst_port; > > > > + > > > > /* DP MST stream allocation (payload bandwidth number) */ > > > > struct dc_dp_mst_stream_allocation { > > > > uint8_t vcp_id; > > > > /* number of slots required for the DP stream in > > > > * transport packet */ > > > > uint8_t slot_count; > > > > + /* The MST port this is on, this is used to associate DC MST > > > > + payloads > > > > with their > > > > + * respective DRM payloads allocations, and can be ignored on non- > > > > Linux. > > > > + */ > > > > > > Is it necessary for adding this new member? Since this is for setting > > > the DC HW and not relating to drm. > > > > I don't entirely know, honestly. The reasons I did it: > > > > * Mapping things from DRM to DC and from DC to DRM is really confusing for > > outside contributors like myself, so it wasn't even really clear to me if > > there was another way to reconstruct the DRM context from the spots > > where > > we call from DC up to DM (not a typo, see next point). > > * These DC structs for MST are already layer mixing as far as I can tell, > > just not in an immediately obvious way. While this struct itself is for > > DC, > > there's multiple spots where we pass the DC payload structs down from > > DM to > > DC, then pass them back up from DC to DM and have to figure out how to > > reconstruct the DRM context that we actually need to use the MST helpers > > from that. So, that kind of further complicates the confusion of where > > layers should be separated. > > * As far as I'm aware with C there shouldn't be any issue with adding a > > pointer to a struct whose contents are undefined. IMHO, this is also > > preferable to just using void* because then at least you get some hint as > > to the actual type of the data and avoid the possibility of casting it to > > the wrong type. So tl;dr, on any platform even outside of Linux with a > > reasonably compliant compiler this should still build just fine. It'll > > even > > give you the added bonus of warning people if they try to access the > > contents of this member in DC on non-Linux platforms. If void* is > > preferred > > though I'm fine with switching it to that. > > > > -- > > Cheers, Lyude Paul (she/her) Software Engineer at Red Hat > > Hi Lyude, > > Thanks for your time! > I was thinking that our DC just mainly takes care of HW related programming. > Struct dc_dp_mst_stream_allocation is only used to construct a copy of the > virtual > channel payload ID and slots count of payload allocation table determined by > dm/drm. ID and slots are only things required for programming HW registers. > I think there shouldn't be any spots to try to construct the DRM context from > this dc struct since there is no such concept in HW level. Our HW should only > take care of local DP link and it doesn't have overall topology info.
Looking at the code I wrote again and realizing I slightly misspoke, looking at the code again I think I probably can drop this. It's likely I just got totally lost with the DC codebase and thought this was necessary when it wasn't. Will drop in the respin > > Thanks! > > Regards, > Wayne -- Cheers, Lyude Paul (she/her) Software Engineer at Red Hat
