Re: [PATCHv2] drm/sun4i: validate modes for HDMI

2017-12-15 Thread Maxime Ripard 
Hi Jose,

On Wed, Dec 13, 2017 at 12:07:05PM +, 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 
> >>> ---
> >>>  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 > 16500)
> >>> + 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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2] drm/sun4i: validate modes for HDMI

2017-12-13 Thread Jose Abreu 
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 
>>> ---
>>>  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 > 16500)
>>> +   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=DwIGaQ=DPL6_X_6JkXFx7AXWqB0tg=yaVFU4TjGY0gVF8El1uKcisy6TPsyCl9uN7Wsis-qhY=HwJwBnLWatgtXZ6tmlyN-ibL-Cj4hheMGPCEW6rh7sY=0f011r4qaTVGDQc5-niD-Faj5yc2kl6jq2KtxKdU20o=

___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2] drm/sun4i: validate modes for HDMI

2017-12-13 Thread Hans Verkuil 
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 
>> ---
>>  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 > 16500)
>> +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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCHv2] drm/sun4i: validate modes for HDMI

2017-12-13 Thread Maxime Ripard 
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 
> ---
>  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 > 16500)
> + 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

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel