On Wed, 2024-12-04 at 15:44 +0100, AngeloGioacchino Del Regno wrote: > External email : Please do not click links or open attachments until you have verified the sender or the content. > > > Il 02/12/24 08:27, CK Hu (胡俊光) ha scritto: > > Hi, Angelo: > > > > On Wed, 2024-11-20 at 13:45 +0100, AngeloGioacchino Del Regno wrote: > > > External email : Please do not click links or open attachments until you have verified the sender or the content. > > > > > > > > > Add support for the newer HDMI-TX (Encoder) v2 and DDC v2 IPs > > > found in MediaTek's MT8195, MT8188 SoC and their variants, and > > > including support for display modes up to 4k60 and for HDMI > > > Audio, as per the HDMI 2.0 spec. > > > > > > HDCP and CEC functionalities are also supported by this hardware, > > > but are not included in this commit. > > > > > > Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> > > > --- > > > > [snip] > > > > > +static int mtk_hdmi_v2_enable(struct mtk_hdmi *hdmi) > > > +{ > > > + int ret; > > > + > > > + ret = pm_runtime_resume_and_get(hdmi->dev); > > > > Why turn on power when attach? > > Should hot-plug detection be always on so power on when attach? > > If so, maybe we need to use CEC to detect hot-plug because CEC has lower power and HDMI power would not be always on. > > As you describe previously, CEC just need some hack to support both HDMI Tx and HDMI Rx. > > So there is a software solution to use CEC to detect hot-plug in a lower power mode. > > And v1 use CEC to detect hot-plug, so it's worth to make v1 and v2 has the same behavior. > > > > As I said, we can't use CEC to detect hotplug, because the same CEC controller > can be used for both HDMI TX and RX at the same time. > > If you use CEC to detect cable attach and you have both HDMI RX and HDMI TX drivers > probed and ready, if you attach the cable to HDMI RX port, HDMI TX will try to get > enabled and will error out. > Same happens if you attach a cable to HDMI TX: HDMI RX will try to get enabled and > will error out. > > The only way to use CEC for hotplug detection is to detect twice: CEC for HDMI > TX/RX controllers wakeup, then reset both, wait for autodetect in both controllers, > suppress errors, and start signal transmit or receive. > > While it is technically possible to do so, it's very complicated and hacky. > > Please keep in mind that on the new SoCs (8188/8195) the CEC block is different > from the one in the old SoCs (6795, 8173, etc) and requires an entirely new driver > to get it working. > > Using CEC for hotplug detection would mean that we need 3 full drivers: > 1. HDMI TX > 2. HDMI RX > 3. CEC > > This is because the CEC is shared, so we need to have all three to properly test > the functionality - and this is not possible to do all at once. > > To save some power (not much, though!), we can eventually go for your proposed > CEC detection *in the future*, but really not right now - and we can do that only > after upstreaming both HDMI TX and HDMI RX drivers. > > The plan (time/effort pemitting!) would look like this: > > Series 1 - Upstream HDMIv2 TX (HPD through HDMIv2 TX controller) - drivers/gpu/drm > Series 2 - Upstream HDMIv2 RX (HPD through HDMIv2 RX controller) - drivers/media > Series 3 - Upstream CEC driver (No HPD in CEC) - drivers/media > Series 4 - Implement HPD in CEC and use it in HDMIv2 TX and HDMIv2 RX - media,drm > > You can see how complicated this is - and I'm sure that you understand why that > cannot be done all at once. > > Adding up some: the HDMI driver is used only on IoT boards for now, which typically > run off AC and not battery - so the (again) very little power consumption raise is > not critical for now. If you plan to upstream HDMI RX driver recently, it's OK not using CEC to detect hot-plug. But for any behavior which is different with v1 and it's related to CEC, add comment about CEC. Regards, CK > > Cheers, > Angelo > > > Regards, > > CK > > > > > > > + if (ret) { > > > + dev_err(hdmi->dev, "Cannot resume HDMI\n"); > > > + return ret; > > > + } > > > + > > > + mtk_hdmi_v2_clk_enable(hdmi); > > > + mtk_hdmi_hw_reset(hdmi); > > > + mtk_hdmi_set_sw_hpd(hdmi, true); > > > + > > > + return 0; > > > +} > > > +