On Wed, 2025-08-27 at 13:05 -0400, Alex Deucher wrote:
> On Wed, Aug 27, 2025 at 12:54 PM Timur Kristóf
> <timur.kris...@gmail.com> wrote:
> >
> > On Wed, 2025-08-27 at 12:36 -0400, Alex Deucher wrote:
> > > On Wed, Aug 27, 2025 at 10:36 AM Timur Kristóf
> > > <timur.kris...@gmail.com> wrote:
> > > >
> > > > On Tue, 2025-08-26 at 10:03 -0400, Alex Deucher wrote:
> > > > > On Mon, Aug 25, 2025 at 6:10 PM Timur Kristóf
> > > > > <timur.kris...@gmail.com> wrote:
> > > > > >
> > > > > > Change the legacy (non-DC) display code to respect the
> > > > > > maximum
> > > > > > pixel clock for HDMI and DVI-D. Reject modes that would
> > > > > > require
> > > > > > a higher pixel clock than can be supported.
> > > > > >
> > > > > > Also update the maximum supported HDMI clock value
> > > > > > depending on
> > > > > > the ASIC type.
> > > > > >
> > > > > > For reference, see the DC code:
> > > > > > check max_hdmi_pixel_clock in dce*_resource.c
> > > > > >
> > > > > > Signed-off-by: Timur Kristóf <timur.kris...@gmail.com>
> > > > > > ---
> > > > > > .../gpu/drm/amd/amdgpu/amdgpu_connectors.c | 21
> > > > > > +++++++++++++++++--
> > > > > > 1 file changed, 19 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> > > > > > b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> > > > > > index 5e375e9c4f5d..abcc4469cf57 100644
> > > > > > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> > > > > > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_connectors.c
> > > > > > @@ -1195,12 +1195,26 @@ static void
> > > > > > amdgpu_connector_dvi_force(struct drm_connector *connector)
> > > > > > amdgpu_connector->use_digital = true;
> > > > > > }
> > > > > >
> > > > > > +/**
> > > > > > + * Returns the maximum supported HDMI (TMDS) pixel clock
> > > > > > in
> > > > > > KHz.
> > > > > > + */
> > > > > > +static int amdgpu_max_hdmi_pixel_clock(const struct
> > > > > > amdgpu_device
> > > > > > *adev)
> > > > > > +{
> > > > > > + if (adev->asic_type >= CHIP_POLARIS10)
> > > > > > + return 600000;
> > > > > > + else if (adev->asic_type >= CHIP_TONGA)
> > > > > > + return 300000;
> > > > > > + else
> > > > > > + return 297000;
> > > > > > +}
> > > > > > +
> > > > > > static enum drm_mode_status
> > > > > > amdgpu_connector_dvi_mode_valid(struct
> > > > > > drm_connector *connector,
> > > > > > const struct
> > > > > > drm_display_mode *mode)
> > > > > > {
> > > > > > struct drm_device *dev = connector->dev;
> > > > > > struct amdgpu_device *adev = drm_to_adev(dev);
> > > > > > struct amdgpu_connector *amdgpu_connector =
> > > > > > to_amdgpu_connector(connector);
> > > > > > + const int max_hdmi_pixel_clock =
> > > > > > amdgpu_max_hdmi_pixel_clock(adev);
> > > > > >
> > > > > > /* XXX check mode bandwidth */
> > > > > >
> > > > > > @@ -1208,10 +1222,13 @@ static enum drm_mode_status
> > > > > > amdgpu_connector_dvi_mode_valid(struct drm_connector
> > > > > > if ((amdgpu_connector->connector_object_id
> > > > > > ==
> > > > > > CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_I) ||
> > > > > > (amdgpu_connector->connector_object_id
> > > > > > ==
> > > > > > CONNECTOR_OBJECT_ID_DUAL_LINK_DVI_D) ||
> > > > > > (amdgpu_connector->connector_object_id
> > > > > > ==
> > > > > > CONNECTOR_OBJECT_ID_HDMI_TYPE_B)) {
> > > > > > - return MODE_OK;
> > > > > > + if (mode->clock >
> > > > > > max_hdmi_pixel_clock)
> > > > > > + return MODE_CLOCK_HIGH;
> > > > > > + else
> > > > > > + return MODE_OK;
> > > > >
> > > > > I don't think it makes sense to limit dual link DVI to the
> > > > > max
> > > > > HDMI
> > > > > clock. HDMI is single link TMDS. Other than that, the patch
> > > > > looks
> > > > > good to me.
> > > > >
> > > > > Alex
> > > >
> > > > Hi Alex,
> > > >
> > > > The problem I'm trying to solve here is that on the non-DC
> > > > code, by
> > > > default I get a black screen on DVI-D because the code doesn't
> > > > set
> > > > an
> > > > upper limit on what is actually supported. (And the monitor
> > > > supports
> > > > some modes which won't work.)
> > > >
> > > > As far as I see, DC only allows 4K 30 Hz on DVI for the same
> > > > connector
> > > > and DVI/HDMI adapter. That may be wrong, though.
> > > >
> > > > What value do you suggest to use?
> > > >
> > > > If HDMI = single link TMDS, does that mean that a passive
> > > > DVI/HDMI
> > > > adapter cannot take advantage of dual-link DVI?
> > >
> > > DVI has a 165 Mhz limit per link so dual link would be 330 Mhz.
> > >
> > > Alex
> >
> > I see. That's my mistake, I thought that the max_hdmi_pixel_clock
> > as
> > defined in DC is applicable to all TMDS signals, not just HDMI.
> >
> > That being said, can you confirm that a DVI/HDMI adapter can indeed
> > use
> > both links of the dual-link DVI?
> >
>
> DVI to HDMI adapters probably only wire up the a single TMDS link
> because that is all HDMI supports. HDMI is single link TMDS and it
> was extended beyond 165 Mhz, so most monitors that support HDMI
> support faster single link clocks on DVI as well if they have both
> inputs since it's the same panel. Older DVI only monitors are
> probably the only ones where the single vs dual link clocks are
> important.
>
> Alex
Would it make sense to still use the HDMI clock for DVI-D when the
monitor is a HDMI monitor, ie. display_info.is_hdmi is set? Considering
that in this case we use the HDMI encoder mode.
>
> > >
> > > >
> > > > Thanks,
> > > > Timur
> > > >
> > > > >
> > > > > > } else if (connector->display_info.is_hdmi)
> > > > > > {
> > > > > > /* HDMI 1.3+ supports max clock of
> > > > > > 340
> > > > > > Mhz
> > > > > > */
> > > > > > - if (mode->clock > 340000)
> > > > > > + if (mode->clock >
> > > > > > max_hdmi_pixel_clock)
> > > > > > return MODE_CLOCK_HIGH;
> > > > > > else
> > > > > > return MODE_OK;
> > > > > > --
> > > > > > 2.50.1
> > > > > >
