| Message ID | 20210317154352.732095-1-maxime@cerno.tech (mailing list archive) |
|---|---|
| Headers | show |
| Series | drm/vc4: hdmi: Add Support for the YUV output | expand |
Hi! Dne sreda, 17. marec 2021 ob 16:43:34 CET je Maxime Ripard napisal(a): > Hi, > > Here's an attempt at support the HDMI YUV output on the BCM2711 SoC found on > the RaspberryPi4. > > I took the same approach than what dw-hdmi did already, turning a bunch of > functions found in that driver into helpers since they were fairly generic. > > However, it feels a bit clunky overall and there's a few rough edges that > should be addressed in a generic manner: > > - while the format negociation makes sense for a bridge, it feels a bit > over-engineered for a simple encoder where that setting could be a simple > switch (and possibly a property?) Property could work, but possible values should be then limited to cross section of HW and connected display capabilities. > > - more importantly, whether we're choosing an YUV output or not is completely > hidden away from the userspace even though it might have some effect on the > visual quality output (thinking about YUV420 and YUV422 here mostly). IMO driver should select highest achievable quality. So in case of YUV420 and YUV422, later should be selected. This should be the case even if the property is implemented. Best regards, Jernej > > - Similarly, the list we report is static and the userspace cannot change or > force one mode over the other. We will always pick YUV444 over RGB444 if > both are available for example. > > While the first one might just be due to a lack of helpers, the second and > third ones are also feeling a bit inconsistent with how we're handling the > 10/12 bit output for example > > Let me know what you think, > Maxime > > Maxime Ripard (18): > drm: Introduce new HDMI helpers > drm/bridge: Add HDMI output fmt helper > drm/bridge: dw-hdmi: Use helpers > drm/vc4: txp: Properly set the possible_crtcs mask > drm/vc4: crtc: Skip the TXP > drm/vc4: Rework the encoder retrieval code > drm/vc4: hdmi: Add full range RGB helper > drm/vc4: hdmi: Use full range helper in csc functions > drm/vc4: hdmi: Remove limited_rgb_range > drm/vc4: hdmi: Convert to bridge > drm/vc4: hdmi: Move XBAR setup to csc_setup > drm/vc4: hdmi: Replace CSC_CTL hardcoded value by defines > drm/vc4: hdmi: Define colorspace matrices > drm/vc4: hdmi: Change CSC callback prototype > drm/vc4: hdmi: Rework the infoframe prototype > drm/vc4: hdmi: Support HDMI YUV output > drm/vc4: hdmi: Move the pixel rate calculation to a helper > drm/vc4: hdmi: Force YUV422 if the rate is too high > > drivers/gpu/drm/Makefile | 2 +- > drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 268 ++------------- > drivers/gpu/drm/drm_bridge.c | 118 +++++++ > drivers/gpu/drm/drm_hdmi.c | 170 +++++++++ > drivers/gpu/drm/vc4/vc4_crtc.c | 59 +++- > drivers/gpu/drm/vc4/vc4_drv.c | 41 +++ > drivers/gpu/drm/vc4/vc4_drv.h | 26 +- > drivers/gpu/drm/vc4/vc4_hdmi.c | 399 +++++++++++++++------- > drivers/gpu/drm/vc4/vc4_hdmi.h | 13 +- > drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 6 + > drivers/gpu/drm/vc4/vc4_regs.h | 19 ++ > drivers/gpu/drm/vc4/vc4_txp.c | 2 +- > include/drm/drm_bridge.h | 6 + > include/drm/drm_hdmi.h | 24 ++ > 14 files changed, 770 insertions(+), 383 deletions(-) > create mode 100644 drivers/gpu/drm/drm_hdmi.c > create mode 100644 include/drm/drm_hdmi.h > > -- > 2.30.2 > >
Hi Jernej, On Thu, Mar 18, 2021 at 07:16:33PM +0100, Jernej Škrabec wrote: > Dne sreda, 17. marec 2021 ob 16:43:34 CET je Maxime Ripard napisal(a): > > Hi, > > > > Here's an attempt at support the HDMI YUV output on the BCM2711 SoC found on > > the RaspberryPi4. > > > > I took the same approach than what dw-hdmi did already, turning a bunch of > > functions found in that driver into helpers since they were fairly generic. > > > > However, it feels a bit clunky overall and there's a few rough edges that > > should be addressed in a generic manner: > > > > - while the format negociation makes sense for a bridge, it feels a bit > > over-engineered for a simple encoder where that setting could be a > simple > > switch (and possibly a property?) > > Property could work, but possible values should be then limited to cross > section of HW and connected display capabilities. That's a good point. I'm not sure if the userspace should expect the list of values of an enum to change under its feet > > - more importantly, whether we're choosing an YUV output or not is > > completely hidden away from the userspace even though it might > > have some effect on > the visual quality output (thinking about > > YUV420 and YUV422 here mostly). > > IMO driver should select highest achievable quality. So in case of > YUV420 and YUV422, later should be selected. This should be the case > even if the property is implemented. Well, it depends on the hardware capability. On RPi4 in some situations (high bpc count), we can't output anything else than YUV422. I'd expect to have some way for the userspace to know at least. And then, for subsampling formats it's fairly easy to tell which is the highest achievable quality, but it would be pretty hard for YUV444 vs RGB? Maxime
On 18/03/2021 19:16, Jernej Škrabec wrote: > Hi! > > Dne sreda, 17. marec 2021 ob 16:43:34 CET je Maxime Ripard napisal(a): >> Hi, >> >> Here's an attempt at support the HDMI YUV output on the BCM2711 SoC found on >> the RaspberryPi4. >> >> I took the same approach than what dw-hdmi did already, turning a bunch of >> functions found in that driver into helpers since they were fairly generic. >> >> However, it feels a bit clunky overall and there's a few rough edges that >> should be addressed in a generic manner: >> >> - while the format negociation makes sense for a bridge, it feels a bit >> over-engineered for a simple encoder where that setting could be a > simple >> switch (and possibly a property?) > > Property could work, but possible values should be then limited to cross > section of HW and connected display capabilities. > >> >> - more importantly, whether we're choosing an YUV output or not is > completely >> hidden away from the userspace even though it might have some effect on > the >> visual quality output (thinking about YUV420 and YUV422 here mostly). > > IMO driver should select highest achievable quality. So in case of YUV420 and > YUV422, later should be selected. This should be the case even if the property > is implemented. > > Best regards, > Jernej > >> >> - Similarly, the list we report is static and the userspace cannot change > or >> force one mode over the other. We will always pick YUV444 over RGB444 if >> both are available for example. >> >> While the first one might just be due to a lack of helpers, the second and >> third ones are also feeling a bit inconsistent with how we're handling the >> 10/12 bit output for example Another points for YUV422 and YUV420 are: - mandatory YUV420 for pre-HDMI2 displays to achieve 4k60 with HDMI1.4 max TDMS - possibility to achieve factorial frequencies for 10/12bits, it's not the case for YUV422, it's the same TMDS character rate for 8, 19, 12 and 16bits - selecting YUV422 instead of YUV444 for 10/12/16 for 4k60 in HDMI2.0 Today we do not take in account the SCDC feedback from the display, but at some point we should monitor the Scrambling_Status and Character Error Detection to lower down from YUV444 to 422 and 420 for example. Neil >> >> Let me know what you think, >> Maxime >> >> Maxime Ripard (18): >> drm: Introduce new HDMI helpers >> drm/bridge: Add HDMI output fmt helper >> drm/bridge: dw-hdmi: Use helpers >> drm/vc4: txp: Properly set the possible_crtcs mask >> drm/vc4: crtc: Skip the TXP >> drm/vc4: Rework the encoder retrieval code >> drm/vc4: hdmi: Add full range RGB helper >> drm/vc4: hdmi: Use full range helper in csc functions >> drm/vc4: hdmi: Remove limited_rgb_range >> drm/vc4: hdmi: Convert to bridge >> drm/vc4: hdmi: Move XBAR setup to csc_setup >> drm/vc4: hdmi: Replace CSC_CTL hardcoded value by defines >> drm/vc4: hdmi: Define colorspace matrices >> drm/vc4: hdmi: Change CSC callback prototype >> drm/vc4: hdmi: Rework the infoframe prototype >> drm/vc4: hdmi: Support HDMI YUV output >> drm/vc4: hdmi: Move the pixel rate calculation to a helper >> drm/vc4: hdmi: Force YUV422 if the rate is too high >> >> drivers/gpu/drm/Makefile | 2 +- >> drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 268 ++------------- >> drivers/gpu/drm/drm_bridge.c | 118 +++++++ >> drivers/gpu/drm/drm_hdmi.c | 170 +++++++++ >> drivers/gpu/drm/vc4/vc4_crtc.c | 59 +++- >> drivers/gpu/drm/vc4/vc4_drv.c | 41 +++ >> drivers/gpu/drm/vc4/vc4_drv.h | 26 +- >> drivers/gpu/drm/vc4/vc4_hdmi.c | 399 +++++++++++++++------- >> drivers/gpu/drm/vc4/vc4_hdmi.h | 13 +- >> drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 6 + >> drivers/gpu/drm/vc4/vc4_regs.h | 19 ++ >> drivers/gpu/drm/vc4/vc4_txp.c | 2 +- >> include/drm/drm_bridge.h | 6 + >> include/drm/drm_hdmi.h | 24 ++ >> 14 files changed, 770 insertions(+), 383 deletions(-) >> create mode 100644 drivers/gpu/drm/drm_hdmi.c >> create mode 100644 include/drm/drm_hdmi.h >> >> -- >> 2.30.2 >> >> > >
Hi, Here's an attempt at support the HDMI YUV output on the BCM2711 SoC found on the RaspberryPi4. I took the same approach than what dw-hdmi did already, turning a bunch of functions found in that driver into helpers since they were fairly generic. However, it feels a bit clunky overall and there's a few rough edges that should be addressed in a generic manner: - while the format negociation makes sense for a bridge, it feels a bit over-engineered for a simple encoder where that setting could be a simple switch (and possibly a property?) - more importantly, whether we're choosing an YUV output or not is completely hidden away from the userspace even though it might have some effect on the visual quality output (thinking about YUV420 and YUV422 here mostly). - Similarly, the list we report is static and the userspace cannot change or force one mode over the other. We will always pick YUV444 over RGB444 if both are available for example. While the first one might just be due to a lack of helpers, the second and third ones are also feeling a bit inconsistent with how we're handling the 10/12 bit output for example Let me know what you think, Maxime Maxime Ripard (18): drm: Introduce new HDMI helpers drm/bridge: Add HDMI output fmt helper drm/bridge: dw-hdmi: Use helpers drm/vc4: txp: Properly set the possible_crtcs mask drm/vc4: crtc: Skip the TXP drm/vc4: Rework the encoder retrieval code drm/vc4: hdmi: Add full range RGB helper drm/vc4: hdmi: Use full range helper in csc functions drm/vc4: hdmi: Remove limited_rgb_range drm/vc4: hdmi: Convert to bridge drm/vc4: hdmi: Move XBAR setup to csc_setup drm/vc4: hdmi: Replace CSC_CTL hardcoded value by defines drm/vc4: hdmi: Define colorspace matrices drm/vc4: hdmi: Change CSC callback prototype drm/vc4: hdmi: Rework the infoframe prototype drm/vc4: hdmi: Support HDMI YUV output drm/vc4: hdmi: Move the pixel rate calculation to a helper drm/vc4: hdmi: Force YUV422 if the rate is too high drivers/gpu/drm/Makefile | 2 +- drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 268 ++------------- drivers/gpu/drm/drm_bridge.c | 118 +++++++ drivers/gpu/drm/drm_hdmi.c | 170 +++++++++ drivers/gpu/drm/vc4/vc4_crtc.c | 59 +++- drivers/gpu/drm/vc4/vc4_drv.c | 41 +++ drivers/gpu/drm/vc4/vc4_drv.h | 26 +- drivers/gpu/drm/vc4/vc4_hdmi.c | 399 +++++++++++++++------- drivers/gpu/drm/vc4/vc4_hdmi.h | 13 +- drivers/gpu/drm/vc4/vc4_hdmi_regs.h | 6 + drivers/gpu/drm/vc4/vc4_regs.h | 19 ++ drivers/gpu/drm/vc4/vc4_txp.c | 2 +- include/drm/drm_bridge.h | 6 + include/drm/drm_hdmi.h | 24 ++ 14 files changed, 770 insertions(+), 383 deletions(-) create mode 100644 drivers/gpu/drm/drm_hdmi.c create mode 100644 include/drm/drm_hdmi.h