| Message ID | 3762ba00-f00e-9709-e57c-0f07128c751c@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Hans, On Fri, Dec 08, 2017 at 04:48:47PM +0100, Hans Verkuil wrote: > When I connected my cubieboard running 4.15-rc1 to my 4k display I got no picture. Some > digging found that there is no check against the upper pixelclock limit of the HDMI > output, so X selects a 4kp60 format at 594 MHz, which obviously won't work. > > The patch below adds a check for the upper bound of what this hardware can do, and > it checks if the requested tmds clock can be obtained. > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c > index dda904ec0534..c10400a19b33 100644 > --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c > +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c > @@ -208,8 +208,27 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector) > return ret; > } > > +static int sun4i_hdmi_mode_valid(struct drm_connector *connector, > + struct drm_display_mode *mode) > +{ > + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); > + unsigned long rate = mode->clock * 1000; > + long rounded_rate; > + > + /* 165 MHz is the typical max pixelclock frequency for HDMI <= 1.2 */ > + if (rate > 165000000) > + return MODE_CLOCK_HIGH; > + rounded_rate = clk_round_rate(hdmi->tmds_clk, rate); > + if (rounded_rate < rate) > + return MODE_CLOCK_LOW; > + if (rounded_rate > rate) > + return MODE_CLOCK_HIGH; > + return MODE_OK; > +} This looks much better, thanks! One thing that I was mentionning in my other mail is that our rate rounding might not provide the exact TMDS clock rate advertised by the EDID, while staying in the tolerancy. We've raised this issue before, without coming to a conclusion. Would you happen to know what that tolerancy would be on an HDMI link? Thanks! Maxime
On 13/12/17 11:30, Maxime Ripard wrote: > Hi Hans, > > On Fri, Dec 08, 2017 at 04:48:47PM +0100, Hans Verkuil wrote: >> When I connected my cubieboard running 4.15-rc1 to my 4k display I got no picture. Some >> digging found that there is no check against the upper pixelclock limit of the HDMI >> output, so X selects a 4kp60 format at 594 MHz, which obviously won't work. >> >> The patch below adds a check for the upper bound of what this hardware can do, and >> it checks if the requested tmds clock can be obtained. >> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> >> --- >> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 19 +++++++++++++++++++ >> 1 file changed, 19 insertions(+) >> >> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >> index dda904ec0534..c10400a19b33 100644 >> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >> @@ -208,8 +208,27 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector) >> return ret; >> } >> >> +static int sun4i_hdmi_mode_valid(struct drm_connector *connector, >> + struct drm_display_mode *mode) >> +{ >> + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); >> + unsigned long rate = mode->clock * 1000; >> + long rounded_rate; >> + >> + /* 165 MHz is the typical max pixelclock frequency for HDMI <= 1.2 */ >> + if (rate > 165000000) >> + return MODE_CLOCK_HIGH; >> + rounded_rate = clk_round_rate(hdmi->tmds_clk, rate); >> + if (rounded_rate < rate) >> + return MODE_CLOCK_LOW; >> + if (rounded_rate > rate) >> + return MODE_CLOCK_HIGH; >> + return MODE_OK; >> +} > > This looks much better, thanks! > > One thing that I was mentionning in my other mail is that our rate > rounding might not provide the exact TMDS clock rate advertised by the > EDID, while staying in the tolerancy. > > We've raised this issue before, without coming to a conclusion. Would > you happen to know what that tolerancy would be on an HDMI link? I can't actually find anything about that in the HDMI spec. However, the VESA DMT spec specifies a tolerance of +/- 0.5% for the pixelclock. The HDMI 2.1 spec also suggests that this is the tolerance (caveat: I have not had the time to study this in detail, but it does mention it when describing the new variable refresh rate feature). That said, the problem with a 0.5% tolerance is that the slight slowdown for 59.94 vs 60 Hz framerate falls within that tolerance (it's a 0.1% slowdown). Generally clocks will be able to hit the standard frequencies (74.25, 148.5, etc) exactly, but if you want to slow down for 59.94 framerate they tend to be off by a bit. In the end I think keeping a margin of 0.4 or 0.5% is the best approach. Regards, Hans
Hi, On 13-12-2017 11:05, Hans Verkuil wrote: > On 13/12/17 11:30, Maxime Ripard wrote: >> Hi Hans, >> >> On Fri, Dec 08, 2017 at 04:48:47PM +0100, Hans Verkuil wrote: >>> When I connected my cubieboard running 4.15-rc1 to my 4k display I got no picture. Some >>> digging found that there is no check against the upper pixelclock limit of the HDMI >>> output, so X selects a 4kp60 format at 594 MHz, which obviously won't work. >>> >>> The patch below adds a check for the upper bound of what this hardware can do, and >>> it checks if the requested tmds clock can be obtained. >>> >>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> >>> --- >>> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 19 +++++++++++++++++++ >>> 1 file changed, 19 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >>> index dda904ec0534..c10400a19b33 100644 >>> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >>> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c >>> @@ -208,8 +208,27 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector) >>> return ret; >>> } >>> >>> +static int sun4i_hdmi_mode_valid(struct drm_connector *connector, >>> + struct drm_display_mode *mode) >>> +{ >>> + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); >>> + unsigned long rate = mode->clock * 1000; >>> + long rounded_rate; >>> + >>> + /* 165 MHz is the typical max pixelclock frequency for HDMI <= 1.2 */ >>> + if (rate > 165000000) >>> + return MODE_CLOCK_HIGH; >>> + rounded_rate = clk_round_rate(hdmi->tmds_clk, rate); >>> + if (rounded_rate < rate) >>> + return MODE_CLOCK_LOW; >>> + if (rounded_rate > rate) >>> + return MODE_CLOCK_HIGH; >>> + return MODE_OK; >>> +} >> This looks much better, thanks! >> >> One thing that I was mentionning in my other mail is that our rate >> rounding might not provide the exact TMDS clock rate advertised by the >> EDID, while staying in the tolerancy. >> >> We've raised this issue before, without coming to a conclusion. Would >> you happen to know what that tolerancy would be on an HDMI link? > I can't actually find anything about that in the HDMI spec. However, the VESA DMT > spec specifies a tolerance of +/- 0.5% for the pixelclock. > > The HDMI 2.1 spec also suggests that this is the tolerance (caveat: I have not had > the time to study this in detail, but it does mention it when describing the new > variable refresh rate feature). > > That said, the problem with a 0.5% tolerance is that the slight slowdown for 59.94 > vs 60 Hz framerate falls within that tolerance (it's a 0.1% slowdown). > > Generally clocks will be able to hit the standard frequencies (74.25, 148.5, etc) > exactly, but if you want to slow down for 59.94 framerate they tend to be off by > a bit. > > In the end I think keeping a margin of 0.4 or 0.5% is the best approach. We had the same problem in arcpgu, please check if commit [1] can be used by you. Best Regards, Jose Miguel Abreu [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=22d0be2a557e53a22feb484e8fce255fe09e6ad5 > > Regards, > > Hans > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.freedesktop.org_mailman_listinfo_dri-2Ddevel&d=DwIGaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY&m=HwJwBnLWatgtXZ6tmlyN-ibL-Cj4hheMGPCEW6rh7sY&s=0f011r4qaTVGDQc5-niD-Faj5yc2kl6jq2KtxKdU20o&e=
Hi Jose, On Wed, Dec 13, 2017 at 12:07:05PM +0000, Jose Abreu wrote: > On 13-12-2017 11:05, Hans Verkuil wrote: > > On 13/12/17 11:30, Maxime Ripard wrote: > >> Hi Hans, > >> > >> On Fri, Dec 08, 2017 at 04:48:47PM +0100, Hans Verkuil wrote: > >>> When I connected my cubieboard running 4.15-rc1 to my 4k display I got no picture. Some > >>> digging found that there is no check against the upper pixelclock limit of the HDMI > >>> output, so X selects a 4kp60 format at 594 MHz, which obviously won't work. > >>> > >>> The patch below adds a check for the upper bound of what this hardware can do, and > >>> it checks if the requested tmds clock can be obtained. > >>> > >>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > >>> --- > >>> drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 19 +++++++++++++++++++ > >>> 1 file changed, 19 insertions(+) > >>> > >>> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c > >>> index dda904ec0534..c10400a19b33 100644 > >>> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c > >>> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c > >>> @@ -208,8 +208,27 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector) > >>> return ret; > >>> } > >>> > >>> +static int sun4i_hdmi_mode_valid(struct drm_connector *connector, > >>> + struct drm_display_mode *mode) > >>> +{ > >>> + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); > >>> + unsigned long rate = mode->clock * 1000; > >>> + long rounded_rate; > >>> + > >>> + /* 165 MHz is the typical max pixelclock frequency for HDMI <= 1.2 */ > >>> + if (rate > 165000000) > >>> + return MODE_CLOCK_HIGH; > >>> + rounded_rate = clk_round_rate(hdmi->tmds_clk, rate); > >>> + if (rounded_rate < rate) > >>> + return MODE_CLOCK_LOW; > >>> + if (rounded_rate > rate) > >>> + return MODE_CLOCK_HIGH; > >>> + return MODE_OK; > >>> +} > >> This looks much better, thanks! > >> > >> One thing that I was mentionning in my other mail is that our rate > >> rounding might not provide the exact TMDS clock rate advertised by the > >> EDID, while staying in the tolerancy. > >> > >> We've raised this issue before, without coming to a conclusion. Would > >> you happen to know what that tolerancy would be on an HDMI link? > > I can't actually find anything about that in the HDMI spec. However, the VESA DMT > > spec specifies a tolerance of +/- 0.5% for the pixelclock. > > > > The HDMI 2.1 spec also suggests that this is the tolerance (caveat: I have not had > > the time to study this in detail, but it does mention it when describing the new > > variable refresh rate feature). > > > > That said, the problem with a 0.5% tolerance is that the slight slowdown for 59.94 > > vs 60 Hz framerate falls within that tolerance (it's a 0.1% slowdown). > > > > Generally clocks will be able to hit the standard frequencies (74.25, 148.5, etc) > > exactly, but if you want to slow down for 59.94 framerate they tend to be off by > > a bit. > > > > In the end I think keeping a margin of 0.4 or 0.5% is the best approach. > > We had the same problem in arcpgu, please check if commit [1] can > be used by you. > > Best Regards, > Jose Miguel Abreu > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=22d0be2a557e53a22feb484e8fce255fe09e6ad5 Yeah, something like that would definitely work. I'm not entirely sure whether we need to report whether the clock was too high or too low, but I'm totally fine with something like that. Hans, do you want to resend a new version of this patch? Thanks! Maxime
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c index dda904ec0534..c10400a19b33 100644 --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c @@ -208,8 +208,27 @@ static int sun4i_hdmi_get_modes(struct drm_connector *connector) return ret; } +static int sun4i_hdmi_mode_valid(struct drm_connector *connector, + struct drm_display_mode *mode) +{ + struct sun4i_hdmi *hdmi = drm_connector_to_sun4i_hdmi(connector); + unsigned long rate = mode->clock * 1000; + long rounded_rate; + + /* 165 MHz is the typical max pixelclock frequency for HDMI <= 1.2 */ + if (rate > 165000000) + return MODE_CLOCK_HIGH; + rounded_rate = clk_round_rate(hdmi->tmds_clk, rate); + if (rounded_rate < rate) + return MODE_CLOCK_LOW; + if (rounded_rate > rate) + return MODE_CLOCK_HIGH; + return MODE_OK; +} + static const struct drm_connector_helper_funcs sun4i_hdmi_connector_helper_funcs = { .get_modes = sun4i_hdmi_get_modes, + .mode_valid = sun4i_hdmi_mode_valid, }; static enum drm_connector_status
When I connected my cubieboard running 4.15-rc1 to my 4k display I got no picture. Some digging found that there is no check against the upper pixelclock limit of the HDMI output, so X selects a 4kp60 format at 594 MHz, which obviously won't work. The patch below adds a check for the upper bound of what this hardware can do, and it checks if the requested tmds clock can be obtained. Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> --- drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)