On 2023-06-22 16:04:30, Abhinav Kumar wrote: <snip> > >> Is widebus applicable only to the CMD mode, or video mode can employ it too? > > > > The patch description states that it was not tested on video-mode yet, > > so I assume it will. But this should also be highlighted with a comment > > (e.g. /* XXX: Allow for video-mode once tested/fixed */), _especially_ > > on the check for MIPI_DSI_MODE_VIDEO above. > > Sure, we can leave a comment. Thanks! > > > If I understand this correctly DSC is not working for video mode at all > > on these setups, right? Or no-one was able to test it? I'm inclined to > > request dropping these artifical guards to have as little friction as > > possible when someone starts enabling and testing this - and less > > patches removing artificial bounds in the future. > > > > Noone was able to test it. Like I have said before, we dont have or have > not brought up any DSI DSC panel with video mode. DP will be the first > to validate the video mode path for DSC so even that time we cannot test > DSI with DSC on video mode. > > I think we should find a panel which supports cmd and video mode ( I > believe one of the HDKs does have that ) and bring that one up first to > validate this. > > I believe we should keep this checks with the comment you have > suggested. If someone tests it and then removes it, I am comfortable > with that. > > Till then, I would rather guard that configuration. Sure. On the one hand my suggestion to drop it would be to simplify DSC video-mode "bring-up" and not put up arbitrary barriers, but for distinct optional features like widebus it makes sense to keep them guarded so that a developer can enable them one at a time. I'm just afraid that them being spread far and wide across the codebase makes it hard to find all the places where this guard is in place. Unless it is hopefully one of the current active developers testing video-mode, because we all know what's where now :) - Marijn
