On Tue, Nov 30, 2021 at 09:29:09AM +0100, Daniel Vetter wrote: > On Mon, Nov 29, 2021 at 11:07:41AM +0100, Maxime Ripard wrote: > > On Fri, Nov 26, 2021 at 06:12:42PM +0100, Daniel Vetter wrote: > > > On Fri, Nov 26, 2021 at 04:43:49PM +0100, Maxime Ripard wrote: > > > > Hi Daniel, > > > > > > > > On Fri, Nov 19, 2021 at 05:01:14PM +0100, Daniel Vetter wrote: > > > > > On Thu, Nov 18, 2021 at 11:38:01AM +0100, Maxime Ripard wrote: > > > > > > This is a follow-up of the work to support the interactions between the hotplug > > > > > > and the scrambling support for vc4: > > > > > > > > > > > > https://lore.kernel.org/dri-devel/20210507150515.257424-11-maxime@xxxxxxxxxx/ > > > > > > https://lore.kernel.org/dri-devel/20211025152903.1088803-10-maxime@xxxxxxxxxx/ > > > > > > > > > > > > Ville feedback was that the same discussion happened some time ago for i915, > > > > > > and resulted in a function to do an full disable/enable cycle on reconnection > > > > > > to avoid breaking the HDMI 2.0 spec. > > > > > > > > > > > > This series improves the current scrambling support by adding generic helpers > > > > > > for usual scrambling-related operations, and builds upon them to provide a > > > > > > generic alternative to intel_hdmi_reset_link. > > > > > > > > > > Out of curiosity, can we rebuild intel_hdmi_reset_link on top of these? > > > > > Always better to have two drivers to actually show the helpers help, than > > > > > just one. > > > > > > > > Unfortunately, I don't have any Intel system I can test it on, and it > > > > looks like the changes wouldn't be trivial. > > > > > > > > Maybe we can use dw-hdmi instead? > > > > > > Hm I guess so, maybe Ville can be motivated. Just figured since this seems > > > at least inspired by i915 code. > > > > It's not really the conversion to the new helper that I suspect would be > > hard to do, but rather the usage of the connector state that we do, and > > the scrambling computation, and how it's all tied together in i915. > > > > It seems to be fairly different from drivers that just rely on the > > atomic helpers, and I'm not really confident about changing that. But > > yeah, I guess I can give it a try and rely on the CI. > > Hm yeah if wiring through connector state to the right places is real work > then skip the conversion. But I thought i915 modeset code has also gone > through some of the state rollout that you've done for atomic helpers. I kind of fell into a rabbit hole with this. I looked at other drivers and which one I could convert too. Tegra was easy enough, but dw-hdmi was more interesting. dw-hdmi also has YUV output, and the output format also impacts the TMDS rate and thus whether the scrambler needs to be enabled or not. In parallel, I also worked on the vc4 HDMI YUV output, trying to mimic what i915 is doing. However, the requirements around which YUV output and bpc we support are a bit non-trivial, and I think it should be part of some helpers to avoid as much as possible someone getting it wrong. For reference, i915 is starting with the highest bpc count it can, and tries either RGB or YUV420 for that bpc count. If any succeeds, we stop there, otherwise we repeat with a lower bit count. The iteration over the available bpc and formats should be easy enough, we would just need to have a bit field with supported bpc values and formats and be done with it. The part that would tell if the sink supports the formats and bpc can be generic as well since it's part of the EDID and we parse it already. However, one of the limitation we have to take into account is also the maximum TMDS rate the controller can achieve, and thus if we could output the TMDS clock for a bpc, format and mode triplet. So, if we were to sum up, we'd need: - One bitfield for the formats supported by the connector (redundant with ycbcr420_allowed?) - One bitfield for the bpc supported by the connector - One field to store the maximum TMDS clock the connector can achieve Then, in drm_connector_state, we'd have in addition to what we have so far the format to output for a given commit. Or maybe it's not worth the trouble and I should just stop trying to make this a helper? Maxime
Attachment:
signature.asc
Description: PGP signature
