| Message ID | 20211102145944.259181-3-maxime@cerno.tech |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [01/13] drm/connector: Add define for HDMI 1.4 Maximum Pixel Rate | expand |
On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote: > --- a/include/drm/drm_modes.h > +++ b/include/drm/drm_modes.h > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) > return mode->flags & DRM_MODE_FLAG_3D_MASK; > } > > +/** > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling > + * @mode: DRM display mode > + * > + * Checks if a given display mode requires the scrambling to be enabled. > + * > + * Returns: > + * A boolean stating whether it's required or not. > + */ > +static inline bool > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) > +{ > + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; > +} That's only correct for 8bpc. The actual limit is on the TMDS clock (or rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4 magic for the actually transmitted TMDS clock).
On Thu, Nov 04, 2021 at 05:37:21PM +0200, Ville Syrjälä wrote: > On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote: > > --- a/include/drm/drm_modes.h > > +++ b/include/drm/drm_modes.h > > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) > > return mode->flags & DRM_MODE_FLAG_3D_MASK; > > } > > > > +/** > > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling > > + * @mode: DRM display mode > > + * > > + * Checks if a given display mode requires the scrambling to be enabled. > > + * > > + * Returns: > > + * A boolean stating whether it's required or not. > > + */ > > +static inline bool > > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) > > +{ > > + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; > > +} > > That's only correct for 8bpc. The actual limit is on the TMDS clock (or > rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4 > magic for the actually transmitted TMDS clock). Yeah we might need to add the bus format for the cable here too, to make this complete. -Daniel
On Fri, Nov 05, 2021 at 07:02:30PM +0100, Daniel Vetter wrote: > On Thu, Nov 04, 2021 at 05:37:21PM +0200, Ville Syrjälä wrote: > > On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote: > > > --- a/include/drm/drm_modes.h > > > +++ b/include/drm/drm_modes.h > > > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) > > > return mode->flags & DRM_MODE_FLAG_3D_MASK; > > > } > > > > > > +/** > > > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling > > > + * @mode: DRM display mode > > > + * > > > + * Checks if a given display mode requires the scrambling to be enabled. > > > + * > > > + * Returns: > > > + * A boolean stating whether it's required or not. > > > + */ > > > +static inline bool > > > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) > > > +{ > > > + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; > > > +} > > > > That's only correct for 8bpc. The actual limit is on the TMDS clock (or > > rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4 > > magic for the actually transmitted TMDS clock). > > Yeah we might need to add the bus format for the cable here too, to make > this complete. I don't think we have a usable thing for that on the drm level, so would need to invent something. Oh, and the mode->clock vs. mode->crtc_clock funny business also limits the usability of this approach. So probably just easiest to pass in the the driver calculated TMDS clock instead.
On Fri, Nov 05, 2021 at 08:14:04PM +0200, Ville Syrjälä wrote: > On Fri, Nov 05, 2021 at 07:02:30PM +0100, Daniel Vetter wrote: > > On Thu, Nov 04, 2021 at 05:37:21PM +0200, Ville Syrjälä wrote: > > > On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote: > > > > --- a/include/drm/drm_modes.h > > > > +++ b/include/drm/drm_modes.h > > > > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) > > > > return mode->flags & DRM_MODE_FLAG_3D_MASK; > > > > } > > > > > > > > +/** > > > > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling > > > > + * @mode: DRM display mode > > > > + * > > > > + * Checks if a given display mode requires the scrambling to be enabled. > > > > + * > > > > + * Returns: > > > > + * A boolean stating whether it's required or not. > > > > + */ > > > > +static inline bool > > > > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) > > > > +{ > > > > + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; > > > > +} > > > > > > That's only correct for 8bpc. The actual limit is on the TMDS clock (or > > > rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4 > > > magic for the actually transmitted TMDS clock). > > > > Yeah we might need to add the bus format for the cable here too, to make > > this complete. > > I don't think we have a usable thing for that on the drm level, so > would need to invent something. Oh, and the mode->clock vs. > mode->crtc_clock funny business also limits the usability of this > approach. So probably just easiest to pass in the the driver calculated > TMDS clock instead. If we look at all (I think?) the existing users of scrambling in KMS as of 5.15, only i915 seems to use crtc_clock (which, in retrospect, seems to be the right thing to do), and only i915 and dw-hdmi use an output format, i915 rolling its own, and dw-hdmi using the mbus formats. I think using the mbus format here makes the most sense: i915 already is rolling a whole bunch of other code, and we use the mbus defines for the bridge format enumeration as well which is probably going to have some interaction. I'm not quite sure what to do next though. The whole point of that series is to streamline as much as possible the scrambling and TMDS ratio setup to avoid the duplication we already have in the drivers that support it, every one using the mostly-the-same-but-slightly-different logic to configure it. The mode is fortunately stored in generic structures so it's easy to make that decision. Having to take into account the output format however makes it mandatory to move the bus format in the drm_connector_state(?) structure too. It's already in the bridge_state though, so should we take the final bridge format as the cable format if it's tied to a bridge? Maxime
On Mon, Nov 08, 2021 at 04:58:34PM +0100, Maxime Ripard wrote: > On Fri, Nov 05, 2021 at 08:14:04PM +0200, Ville Syrjälä wrote: > > On Fri, Nov 05, 2021 at 07:02:30PM +0100, Daniel Vetter wrote: > > > On Thu, Nov 04, 2021 at 05:37:21PM +0200, Ville Syrjälä wrote: > > > > On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote: > > > > > --- a/include/drm/drm_modes.h > > > > > +++ b/include/drm/drm_modes.h > > > > > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) > > > > > return mode->flags & DRM_MODE_FLAG_3D_MASK; > > > > > } > > > > > > > > > > +/** > > > > > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling > > > > > + * @mode: DRM display mode > > > > > + * > > > > > + * Checks if a given display mode requires the scrambling to be enabled. > > > > > + * > > > > > + * Returns: > > > > > + * A boolean stating whether it's required or not. > > > > > + */ > > > > > +static inline bool > > > > > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) > > > > > +{ > > > > > + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; > > > > > +} > > > > > > > > That's only correct for 8bpc. The actual limit is on the TMDS clock (or > > > > rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4 > > > > magic for the actually transmitted TMDS clock). > > > > > > Yeah we might need to add the bus format for the cable here too, to make > > > this complete. > > > > I don't think we have a usable thing for that on the drm level, so > > would need to invent something. Oh, and the mode->clock vs. > > mode->crtc_clock funny business also limits the usability of this > > approach. So probably just easiest to pass in the the driver calculated > > TMDS clock instead. > > If we look at all (I think?) the existing users of scrambling in KMS as > of 5.15, only i915 seems to use crtc_clock (which, in retrospect, seems > to be the right thing to do), and only i915 and dw-hdmi use an output > format, i915 rolling its own, and dw-hdmi using the mbus formats. > > I think using the mbus format here makes the most sense: i915 already is > rolling a whole bunch of other code, and we use the mbus defines for the > bridge format enumeration as well which is probably going to have some > interaction. > > I'm not quite sure what to do next though. The whole point of that > series is to streamline as much as possible the scrambling and TMDS > ratio setup to avoid the duplication we already have in the drivers that > support it, every one using the mostly-the-same-but-slightly-different > logic to configure it. > > The mode is fortunately stored in generic structures so it's easy to > make that decision. Having to take into account the output format > however makes it mandatory to move the bus format in the > drm_connector_state(?) structure too. > > It's already in the bridge_state though, so should we take the final > bridge format as the cable format if it's tied to a bridge? Maybe as a default, it nothing is set. Also if nothing is set in the connector then just assume 8bpc rgb, and drivers can be fixed as we go. -Daniel
On Mon, Nov 08, 2021 at 04:58:34PM +0100, Maxime Ripard wrote: > On Fri, Nov 05, 2021 at 08:14:04PM +0200, Ville Syrjälä wrote: > > On Fri, Nov 05, 2021 at 07:02:30PM +0100, Daniel Vetter wrote: > > > On Thu, Nov 04, 2021 at 05:37:21PM +0200, Ville Syrjälä wrote: > > > > On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote: > > > > > --- a/include/drm/drm_modes.h > > > > > +++ b/include/drm/drm_modes.h > > > > > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) > > > > > return mode->flags & DRM_MODE_FLAG_3D_MASK; > > > > > } > > > > > > > > > > +/** > > > > > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling > > > > > + * @mode: DRM display mode > > > > > + * > > > > > + * Checks if a given display mode requires the scrambling to be enabled. > > > > > + * > > > > > + * Returns: > > > > > + * A boolean stating whether it's required or not. > > > > > + */ > > > > > +static inline bool > > > > > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) > > > > > +{ > > > > > + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; > > > > > +} > > > > > > > > That's only correct for 8bpc. The actual limit is on the TMDS clock (or > > > > rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4 > > > > magic for the actually transmitted TMDS clock). > > > > > > Yeah we might need to add the bus format for the cable here too, to make > > > this complete. > > > > I don't think we have a usable thing for that on the drm level, so > > would need to invent something. Oh, and the mode->clock vs. > > mode->crtc_clock funny business also limits the usability of this > > approach. So probably just easiest to pass in the the driver calculated > > TMDS clock instead. > > If we look at all (I think?) the existing users of scrambling in KMS as > of 5.15, only i915 seems to use crtc_clock (which, in retrospect, seems > to be the right thing to do), and only i915 and dw-hdmi use an output > format, i915 rolling its own, and dw-hdmi using the mbus formats. > > I think using the mbus format here makes the most sense: i915 already is > rolling a whole bunch of other code, and we use the mbus defines for the > bridge format enumeration as well which is probably going to have some > interaction. > > I'm not quite sure what to do next though. The whole point of that > series is to streamline as much as possible the scrambling and TMDS > ratio setup to avoid the duplication we already have in the drivers that > support it, every one using the mostly-the-same-but-slightly-different > logic to configure it. > > The mode is fortunately stored in generic structures so it's easy to > make that decision. Having to take into account the output format > however makes it mandatory to move the bus format in the > drm_connector_state(?) structure too. I think involving state objects and the like is just going to make it harder to share code between all drivers, if that is the goal. Just a few tiny helpers I think is what would allow the broadest reuse. I guess you could build additional midlayer on top of those for some drivers if you wish. As for the bus_format stuff, that looks at the same time overkill, and insufficiently documented. I guess its main purpose is to exactly defines how some digtal bus works, which makes sense when you're chaining a bunch of random chips together. But looks overly complicated to me for defining what to output from a HDMI encoder. Looking at the defines I wouldn't even know what to use for HDMI actually. All we really want is RGB 4:4:4 vs. YCbCr 4:4:4 vs. YCbCr 4:2:2 vs. YCbCr 4:2:0. Beyond that level of detail we don't care how each bit gets transferred etc. Hence enum intel_output_format in i915.
On Mon, Nov 08, 2021 at 07:55:00PM +0200, Ville Syrjälä wrote: > On Mon, Nov 08, 2021 at 04:58:34PM +0100, Maxime Ripard wrote: > > On Fri, Nov 05, 2021 at 08:14:04PM +0200, Ville Syrjälä wrote: > > > On Fri, Nov 05, 2021 at 07:02:30PM +0100, Daniel Vetter wrote: > > > > On Thu, Nov 04, 2021 at 05:37:21PM +0200, Ville Syrjälä wrote: > > > > > On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote: > > > > > > --- a/include/drm/drm_modes.h > > > > > > +++ b/include/drm/drm_modes.h > > > > > > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) > > > > > > return mode->flags & DRM_MODE_FLAG_3D_MASK; > > > > > > } > > > > > > > > > > > > +/** > > > > > > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling > > > > > > + * @mode: DRM display mode > > > > > > + * > > > > > > + * Checks if a given display mode requires the scrambling to be enabled. > > > > > > + * > > > > > > + * Returns: > > > > > > + * A boolean stating whether it's required or not. > > > > > > + */ > > > > > > +static inline bool > > > > > > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) > > > > > > +{ > > > > > > + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; > > > > > > +} > > > > > > > > > > That's only correct for 8bpc. The actual limit is on the TMDS clock (or > > > > > rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4 > > > > > magic for the actually transmitted TMDS clock). > > > > > > > > Yeah we might need to add the bus format for the cable here too, to make > > > > this complete. > > > > > > I don't think we have a usable thing for that on the drm level, so > > > would need to invent something. Oh, and the mode->clock vs. > > > mode->crtc_clock funny business also limits the usability of this > > > approach. So probably just easiest to pass in the the driver calculated > > > TMDS clock instead. > > > > If we look at all (I think?) the existing users of scrambling in KMS as > > of 5.15, only i915 seems to use crtc_clock (which, in retrospect, seems > > to be the right thing to do), and only i915 and dw-hdmi use an output > > format, i915 rolling its own, and dw-hdmi using the mbus formats. > > > > I think using the mbus format here makes the most sense: i915 already is > > rolling a whole bunch of other code, and we use the mbus defines for the > > bridge format enumeration as well which is probably going to have some > > interaction. > > > > I'm not quite sure what to do next though. The whole point of that > > series is to streamline as much as possible the scrambling and TMDS > > ratio setup to avoid the duplication we already have in the drivers that > > support it, every one using the mostly-the-same-but-slightly-different > > logic to configure it. > > > > The mode is fortunately stored in generic structures so it's easy to > > make that decision. Having to take into account the output format > > however makes it mandatory to move the bus format in the > > drm_connector_state(?) structure too. > > I think involving state objects and the like is just going to make > it harder to share code between all drivers, if that is the goal. > Just a few tiny helpers I think is what would allow the broadest > reuse. I guess you could build additional midlayer on top of those > for some drivers if you wish. > > As for the bus_format stuff, that looks at the same time overkill, > and insufficiently documented. I guess its main purpose is to exactly > defines how some digtal bus works, which makes sense when you're > chaining a bunch of random chips together. But looks overly complicated > to me for defining what to output from a HDMI encoder. Looking at the > defines I wouldn't even know what to use for HDMI actually. All we > really want is RGB 4:4:4 vs. YCbCr 4:4:4 vs. YCbCr 4:2:2 vs. YCbCr 4:2:0. > Beyond that level of detail we don't care how each bit gets transferred > etc. Hence enum intel_output_format in i915. I have the same feeling about the mbus formats. I tried to start a discussion about this some time back, without much success: https://lore.kernel.org/all/20210317154352.732095-1-maxime@cerno.tech/ The main issue for our current series is that it's tied to the bridges, while it should work for any HDMI connector, backed by a bridge or not. However, the main point of this series is to streamline as much as possible the scrambling setup, including dealing with hotplug properly just like i915 is doing. A flag in the connector state to enable the scrambling and high tmds ratio allows for the helper to perform the disable/enable cycle when we detected the hotplug. If we wouldn't have it, it wouldn't know what to do in the first place, and we would end up disabling/enabling the display every time (which isn't great). This also allows for less duplication since everyone is using a variant of the same algorithm to do so, without proper consideration for corner cases (like enabling scrambling for displays that supports it for rates < 340MHz) So we really need something generic there. Or maybe an intermediate hdmi_connector_state? Maxime
On Mon, Nov 08, 2021 at 07:55:00PM +0200, Ville Syrjälä wrote: > On Mon, Nov 08, 2021 at 04:58:34PM +0100, Maxime Ripard wrote: > > On Fri, Nov 05, 2021 at 08:14:04PM +0200, Ville Syrjälä wrote: > > > On Fri, Nov 05, 2021 at 07:02:30PM +0100, Daniel Vetter wrote: > > > > On Thu, Nov 04, 2021 at 05:37:21PM +0200, Ville Syrjälä wrote: > > > > > On Tue, Nov 02, 2021 at 03:59:33PM +0100, Maxime Ripard wrote: > > > > > > --- a/include/drm/drm_modes.h > > > > > > +++ b/include/drm/drm_modes.h > > > > > > @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) > > > > > > return mode->flags & DRM_MODE_FLAG_3D_MASK; > > > > > > } > > > > > > > > > > > > +/** > > > > > > + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling > > > > > > + * @mode: DRM display mode > > > > > > + * > > > > > > + * Checks if a given display mode requires the scrambling to be enabled. > > > > > > + * > > > > > > + * Returns: > > > > > > + * A boolean stating whether it's required or not. > > > > > > + */ > > > > > > +static inline bool > > > > > > +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) > > > > > > +{ > > > > > > + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; > > > > > > +} > > > > > > > > > > That's only correct for 8bpc. The actual limit is on the TMDS clock (or > > > > > rather TMDS character rate as HDMI 2.0 calls it due to the 1/1 vs. 1/4 > > > > > magic for the actually transmitted TMDS clock). > > > > > > > > Yeah we might need to add the bus format for the cable here too, to make > > > > this complete. > > > > > > I don't think we have a usable thing for that on the drm level, so > > > would need to invent something. Oh, and the mode->clock vs. > > > mode->crtc_clock funny business also limits the usability of this > > > approach. So probably just easiest to pass in the the driver calculated > > > TMDS clock instead. > > > > If we look at all (I think?) the existing users of scrambling in KMS as > > of 5.15, only i915 seems to use crtc_clock (which, in retrospect, seems > > to be the right thing to do), and only i915 and dw-hdmi use an output > > format, i915 rolling its own, and dw-hdmi using the mbus formats. > > > > I think using the mbus format here makes the most sense: i915 already is > > rolling a whole bunch of other code, and we use the mbus defines for the > > bridge format enumeration as well which is probably going to have some > > interaction. > > > > I'm not quite sure what to do next though. The whole point of that > > series is to streamline as much as possible the scrambling and TMDS > > ratio setup to avoid the duplication we already have in the drivers that > > support it, every one using the mostly-the-same-but-slightly-different > > logic to configure it. > > > > The mode is fortunately stored in generic structures so it's easy to > > make that decision. Having to take into account the output format > > however makes it mandatory to move the bus format in the > > drm_connector_state(?) structure too. > > I think involving state objects and the like is just going to make > it harder to share code between all drivers, if that is the goal. > Just a few tiny helpers I think is what would allow the broadest > reuse. I guess you could build additional midlayer on top of those > for some drivers if you wish. > > As for the bus_format stuff, that looks at the same time overkill, > and insufficiently documented. I guess its main purpose is to exactly > defines how some digtal bus works, which makes sense when you're > chaining a bunch of random chips together. But looks overly complicated > to me for defining what to output from a HDMI encoder. Looking at the > defines I wouldn't even know what to use for HDMI actually. All we > really want is RGB 4:4:4 vs. YCbCr 4:4:4 vs. YCbCr 4:2:2 vs. YCbCr 4:2:0. > Beyond that level of detail we don't care how each bit gets transferred > etc. Hence enum intel_output_format in i915. I was thinking about this some more, can we leverage struct hdmi_colorspace for this? Chances are it's already used by any driver that supports YCbCr output to setup the infoframes. Maxime
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c index 99a2d627bfeb..390bd04b0d22 100644 --- a/drivers/gpu/drm/tegra/sor.c +++ b/drivers/gpu/drm/tegra/sor.c @@ -2192,11 +2192,11 @@ static void tegra_sor_hdmi_scdc_work(struct work_struct *work) static void tegra_sor_hdmi_scdc_start(struct tegra_sor *sor) { struct drm_scdc *scdc = &sor->output.connector.display_info.hdmi.scdc; - struct drm_display_mode *mode; + const struct drm_display_mode *mode; mode = &sor->output.encoder.crtc->state->adjusted_mode; - if (mode->clock >= DRM_HDMI_14_MAX_TMDS_CLK_KHZ && scdc->supported) { + if (drm_mode_hdmi_requires_scrambling(mode) && scdc->supported) { schedule_delayed_work(&sor->scdc, msecs_to_jiffies(5000)); tegra_sor_hdmi_scdc_enable(sor); sor->scdc_enabled = true; diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c index fc7247cc1022..fa76ae2c94e4 100644 --- a/drivers/gpu/drm/vc4/vc4_hdmi.c +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c @@ -99,11 +99,6 @@ #define HDMI_14_MAX_TMDS_CLK (DRM_HDMI_14_MAX_TMDS_CLK_KHZ * 1000) -static bool vc4_hdmi_mode_needs_scrambling(const struct drm_display_mode *mode) -{ - return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; -} - static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) { struct drm_info_node *node = (struct drm_info_node *)m->private; @@ -260,10 +255,10 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector) if (vc4_hdmi->disable_4kp60) { struct drm_device *drm = connector->dev; - struct drm_display_mode *mode; + const struct drm_display_mode *mode; list_for_each_entry(mode, &connector->probed_modes, head) { - if (vc4_hdmi_mode_needs_scrambling(mode)) { + if (drm_mode_hdmi_requires_scrambling(mode)) { drm_warn_once(drm, "The core clock cannot reach frequencies high enough to support 4k @ 60Hz."); drm_warn_once(drm, "Please change your config.txt file to add hdmi_enable_4kp60."); } @@ -574,7 +569,7 @@ static void vc4_hdmi_set_infoframes(struct drm_encoder *encoder) } static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder, - struct drm_display_mode *mode) + const struct drm_display_mode *mode) { struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder); struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); @@ -597,7 +592,7 @@ static bool vc4_hdmi_supports_scrambling(struct drm_encoder *encoder, static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) { struct vc4_hdmi *vc4_hdmi = encoder_to_vc4_hdmi(encoder); - struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; + const struct drm_display_mode *mode = &vc4_hdmi->saved_adjusted_mode; unsigned long flags; lockdep_assert_held(&vc4_hdmi->mutex); @@ -605,7 +600,7 @@ static void vc4_hdmi_enable_scrambling(struct drm_encoder *encoder) if (!vc4_hdmi_supports_scrambling(encoder, mode)) return; - if (!vc4_hdmi_mode_needs_scrambling(mode)) + if (!drm_mode_hdmi_requires_scrambling(mode)) return; drm_scdc_set_high_tmds_clock_ratio(vc4_hdmi->ddc, true); @@ -1283,7 +1278,8 @@ static int vc4_hdmi_encoder_atomic_check(struct drm_encoder *encoder, if (pixel_rate > vc4_hdmi->variant->max_pixel_clock) return -EINVAL; - if (vc4_hdmi->disable_4kp60 && (pixel_rate > HDMI_14_MAX_TMDS_CLK)) + if (vc4_hdmi->disable_4kp60 && + drm_mode_hdmi_requires_scrambling(mode)) return -EINVAL; vc4_state->pixel_rate = pixel_rate; @@ -1305,7 +1301,8 @@ vc4_hdmi_encoder_mode_valid(struct drm_encoder *encoder, if ((mode->clock * 1000) > vc4_hdmi->variant->max_pixel_clock) return MODE_CLOCK_HIGH; - if (vc4_hdmi->disable_4kp60 && vc4_hdmi_mode_needs_scrambling(mode)) + if (vc4_hdmi->disable_4kp60 && + drm_mode_hdmi_requires_scrambling(mode)) return MODE_CLOCK_HIGH; return MODE_OK; diff --git a/include/drm/drm_modes.h b/include/drm/drm_modes.h index 29ba4adf0c53..d22816d55836 100644 --- a/include/drm/drm_modes.h +++ b/include/drm/drm_modes.h @@ -424,6 +424,21 @@ static inline bool drm_mode_is_stereo(const struct drm_display_mode *mode) return mode->flags & DRM_MODE_FLAG_3D_MASK; } +/** + * drm_mode_hdmi_requires_scrambling - Checks if a mode requires HDMI Scrambling + * @mode: DRM display mode + * + * Checks if a given display mode requires the scrambling to be enabled. + * + * Returns: + * A boolean stating whether it's required or not. + */ +static inline bool +drm_mode_hdmi_requires_scrambling(const struct drm_display_mode *mode) +{ + return mode->clock > DRM_HDMI_14_MAX_TMDS_CLK_KHZ; +} + struct drm_connector; struct drm_cmdline_mode;
Most drivers supporting the HDMI scrambling seem to have the HDMI 1.4 maximum frequency open-coded, and a function to test whether a display mode is above that threshold to control whether or not scrambling should be enabled. Let's create a common define and helper for drivers to reuse. Cc: Emma Anholt <emma@anholt.net> Cc: Jonathan Hunter <jonathanh@nvidia.com> Cc: linux-tegra@vger.kernel.org Cc: Thierry Reding <thierry.reding@gmail.com> Signed-off-by: Maxime Ripard <maxime@cerno.tech> --- drivers/gpu/drm/tegra/sor.c | 4 ++-- drivers/gpu/drm/vc4/vc4_hdmi.c | 21 +++++++++------------ include/drm/drm_modes.h | 15 +++++++++++++++ 3 files changed, 26 insertions(+), 14 deletions(-)