[v3,5/6] kms/nv50: detect HDMI max MHz correctly

Submitted by Karol Herbst on Aug. 3, 2018, 12:19 p.m.

Details

Message ID 20180803121939.582-6-kherbst@redhat.com
State New
Headers show
Series "improve feature detection" ( rev: 2 ) in Nouveau

Not browsing as part of any series.

Commit Message

Karol Herbst Aug. 3, 2018, 12:19 p.m.
v2: clean up left over comments
    don't overwrite hdmimhz parameter
    cap to 297MHz

Signed-off-by: Karol Herbst <kherbst@redhat.com>
---
 drm/nouveau/dispnv50/disp.c     |  5 +++++
 drm/nouveau/nouveau_connector.c | 15 ++++++++++-----
 drm/nouveau/nouveau_encoder.h   |  4 ++++
 3 files changed, 19 insertions(+), 5 deletions(-)

Patch hide | download patch | download mbox

diff --git a/drm/nouveau/dispnv50/disp.c b/drm/nouveau/dispnv50/disp.c
index fa23d7a2..103433cb 100644
--- a/drm/nouveau/dispnv50/disp.c
+++ b/drm/nouveau/dispnv50/disp.c
@@ -1433,7 +1433,12 @@  nv50_sor_create(struct drm_connector *connector, struct dcb_output *dcbe,
 	case DCB_OUTPUT_LVDS: type = DRM_MODE_ENCODER_LVDS; break;
 	case DCB_OUTPUT_DP:
 		nv_encoder->dp.no_interlace = caps->sor[or].dp.no_interlace;
+		type = DRM_MODE_ENCODER_TMDS;
+		break;
 	case DCB_OUTPUT_TMDS:
+		nv_encoder->tmds.max_mhz = caps->sor[or].tmds.max_mhz;
+		type = DRM_MODE_ENCODER_TMDS;
+		break;
 	default:
 		type = DRM_MODE_ENCODER_TMDS;
 		break;
diff --git a/drm/nouveau/nouveau_connector.c b/drm/nouveau/nouveau_connector.c
index 074e6d52..65fac604 100644
--- a/drm/nouveau/nouveau_connector.c
+++ b/drm/nouveau/nouveau_connector.c
@@ -980,15 +980,20 @@  static unsigned
 get_tmds_link_bandwidth(struct drm_connector *connector, bool hdmi)
 {
 	struct nouveau_connector *nv_connector = nouveau_connector(connector);
+	struct nouveau_encoder *nv_encoder = nv_connector->detected_encoder;
 	struct nouveau_drm *drm = nouveau_drm(connector->dev);
 	struct dcb_output *dcb = nv_connector->detected_encoder->dcb;
 
+	if (hdmi && nouveau_hdmimhz > 0)
+		return nouveau_hdmimhz * 1000;
+
+	if (nv_encoder->tmds.max_mhz)
+		/* no HDMI 2.0 for now and not quite clear if we can use
+                 * higher HDMI clocks than 297MHz
+                 */
+		return min(297, nv_encoder->tmds.max_mhz) * 1000;
+
 	if (hdmi) {
-		if (nouveau_hdmimhz > 0)
-			return nouveau_hdmimhz * 1000;
-		/* Note: these limits are conservative, some Fermi's
-		 * can do 297 MHz. Unclear how this can be determined.
-		 */
 		if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_KEPLER)
 			return 297000;
 		if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_FERMI)
diff --git a/drm/nouveau/nouveau_encoder.h b/drm/nouveau/nouveau_encoder.h
index f74af5ce..fbef9dc0 100644
--- a/drm/nouveau/nouveau_encoder.h
+++ b/drm/nouveau/nouveau_encoder.h
@@ -65,6 +65,10 @@  struct nouveau_encoder {
 			int link_bw;
 			bool no_interlace;
 		} dp;
+
+		struct {
+			uint16_t max_mhz;
+		} tmds;
 	};
 
 	void (*enc_save)(struct drm_encoder *encoder);

Comments

On Fri, Aug 3, 2018 at 8:19 AM, Karol Herbst <kherbst@redhat.com> wrote:
> v2: clean up left over comments
>     don't overwrite hdmimhz parameter
>     cap to 297MHz
>
> Signed-off-by: Karol Herbst <kherbst@redhat.com>
> ---
>  drm/nouveau/dispnv50/disp.c     |  5 +++++
>  drm/nouveau/nouveau_connector.c | 15 ++++++++++-----
>  drm/nouveau/nouveau_encoder.h   |  4 ++++
>  3 files changed, 19 insertions(+), 5 deletions(-)
>
> diff --git a/drm/nouveau/dispnv50/disp.c b/drm/nouveau/dispnv50/disp.c
> index fa23d7a2..103433cb 100644
> --- a/drm/nouveau/dispnv50/disp.c
> +++ b/drm/nouveau/dispnv50/disp.c
> @@ -1433,7 +1433,12 @@ nv50_sor_create(struct drm_connector *connector, struct dcb_output *dcbe,
>         case DCB_OUTPUT_LVDS: type = DRM_MODE_ENCODER_LVDS; break;
>         case DCB_OUTPUT_DP:
>                 nv_encoder->dp.no_interlace = caps->sor[or].dp.no_interlace;
> +               type = DRM_MODE_ENCODER_TMDS;
> +               break;
>         case DCB_OUTPUT_TMDS:
> +               nv_encoder->tmds.max_mhz = caps->sor[or].tmds.max_mhz;
> +               type = DRM_MODE_ENCODER_TMDS;
> +               break;
>         default:
>                 type = DRM_MODE_ENCODER_TMDS;
>                 break;
> diff --git a/drm/nouveau/nouveau_connector.c b/drm/nouveau/nouveau_connector.c
> index 074e6d52..65fac604 100644
> --- a/drm/nouveau/nouveau_connector.c
> +++ b/drm/nouveau/nouveau_connector.c
> @@ -980,15 +980,20 @@ static unsigned
>  get_tmds_link_bandwidth(struct drm_connector *connector, bool hdmi)
>  {
>         struct nouveau_connector *nv_connector = nouveau_connector(connector);
> +       struct nouveau_encoder *nv_encoder = nv_connector->detected_encoder;
>         struct nouveau_drm *drm = nouveau_drm(connector->dev);
>         struct dcb_output *dcb = nv_connector->detected_encoder->dcb;
>
> +       if (hdmi && nouveau_hdmimhz > 0)
> +               return nouveau_hdmimhz * 1000;
> +
> +       if (nv_encoder->tmds.max_mhz)
> +               /* no HDMI 2.0 for now and not quite clear if we can use
> +                 * higher HDMI clocks than 297MHz
> +                 */
> +               return min(297, nv_encoder->tmds.max_mhz) * 1000;

So ... will you start reporting 297MHz single-link TMDS bandwidth over DVI now?

What values show up in max_mhz in practice, across the boards you've
tested (ideally a per-generation summary would be nice).

> +
>         if (hdmi) {
> -               if (nouveau_hdmimhz > 0)
> -                       return nouveau_hdmimhz * 1000;
> -               /* Note: these limits are conservative, some Fermi's
> -                * can do 297 MHz. Unclear how this can be determined.
> -                */
>                 if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_KEPLER)
>                         return 297000;
>                 if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_FERMI)
> diff --git a/drm/nouveau/nouveau_encoder.h b/drm/nouveau/nouveau_encoder.h
> index f74af5ce..fbef9dc0 100644
> --- a/drm/nouveau/nouveau_encoder.h
> +++ b/drm/nouveau/nouveau_encoder.h
> @@ -65,6 +65,10 @@ struct nouveau_encoder {
>                         int link_bw;
>                         bool no_interlace;
>                 } dp;
> +
> +               struct {
> +                       uint16_t max_mhz;
> +               } tmds;
>         };
>
>         void (*enc_save)(struct drm_encoder *encoder);
> --
> 2.17.1
>
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau
On Fri, Aug 3, 2018 at 4:08 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
> On Fri, Aug 3, 2018 at 8:19 AM, Karol Herbst <kherbst@redhat.com> wrote:
>> v2: clean up left over comments
>>     don't overwrite hdmimhz parameter
>>     cap to 297MHz
>>
>> Signed-off-by: Karol Herbst <kherbst@redhat.com>
>> ---
>>  drm/nouveau/dispnv50/disp.c     |  5 +++++
>>  drm/nouveau/nouveau_connector.c | 15 ++++++++++-----
>>  drm/nouveau/nouveau_encoder.h   |  4 ++++
>>  3 files changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/drm/nouveau/dispnv50/disp.c b/drm/nouveau/dispnv50/disp.c
>> index fa23d7a2..103433cb 100644
>> --- a/drm/nouveau/dispnv50/disp.c
>> +++ b/drm/nouveau/dispnv50/disp.c
>> @@ -1433,7 +1433,12 @@ nv50_sor_create(struct drm_connector *connector, struct dcb_output *dcbe,
>>         case DCB_OUTPUT_LVDS: type = DRM_MODE_ENCODER_LVDS; break;
>>         case DCB_OUTPUT_DP:
>>                 nv_encoder->dp.no_interlace = caps->sor[or].dp.no_interlace;
>> +               type = DRM_MODE_ENCODER_TMDS;
>> +               break;
>>         case DCB_OUTPUT_TMDS:
>> +               nv_encoder->tmds.max_mhz = caps->sor[or].tmds.max_mhz;
>> +               type = DRM_MODE_ENCODER_TMDS;
>> +               break;
>>         default:
>>                 type = DRM_MODE_ENCODER_TMDS;
>>                 break;
>> diff --git a/drm/nouveau/nouveau_connector.c b/drm/nouveau/nouveau_connector.c
>> index 074e6d52..65fac604 100644
>> --- a/drm/nouveau/nouveau_connector.c
>> +++ b/drm/nouveau/nouveau_connector.c
>> @@ -980,15 +980,20 @@ static unsigned
>>  get_tmds_link_bandwidth(struct drm_connector *connector, bool hdmi)
>>  {
>>         struct nouveau_connector *nv_connector = nouveau_connector(connector);
>> +       struct nouveau_encoder *nv_encoder = nv_connector->detected_encoder;
>>         struct nouveau_drm *drm = nouveau_drm(connector->dev);
>>         struct dcb_output *dcb = nv_connector->detected_encoder->dcb;
>>
>> +       if (hdmi && nouveau_hdmimhz > 0)
>> +               return nouveau_hdmimhz * 1000;
>> +
>> +       if (nv_encoder->tmds.max_mhz)
>> +               /* no HDMI 2.0 for now and not quite clear if we can use
>> +                 * higher HDMI clocks than 297MHz
>> +                 */
>> +               return min(297, nv_encoder->tmds.max_mhz) * 1000;
>
> So ... will you start reporting 297MHz single-link TMDS bandwidth over DVI now?
>
> What values show up in max_mhz in practice, across the boards you've
> tested (ideally a per-generation summary would be nice).
>

It is a bit silly that those limits are multiple of 10MHz though :/

Fermi (GF119, allthough those caps are there starting with GF110, but
changing the code for that is a bit more messy currently): 230
Kepler: 340
Maxwell+: 600

I really want to test this 340MHz limit on Kepler before though.

we also have a few more caps (which are unrelated to the max clock
over DVI, but maybe you have some ideas where we could use them or fix
potential issues we don't know yet how to fix them?):
SINGLE_LVDS18
SINGLE_LVDS24
DUAL_LVDS18
DUAL_LVDS24
SINGLE_TMDS_A
SINGLE_TMDS_B
DUAL_TMDS
DP_A
DP_B

besides that there is tons of stuff inside
open-gpu-doc/Display-Class-Methods/2/cl907d.h, it is the
CORE_NOTIFIER_3_CAPABILITIES stuff.

>> +
>>         if (hdmi) {
>> -               if (nouveau_hdmimhz > 0)
>> -                       return nouveau_hdmimhz * 1000;
>> -               /* Note: these limits are conservative, some Fermi's
>> -                * can do 297 MHz. Unclear how this can be determined.
>> -                */
>>                 if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_KEPLER)
>>                         return 297000;
>>                 if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_FERMI)
>> diff --git a/drm/nouveau/nouveau_encoder.h b/drm/nouveau/nouveau_encoder.h
>> index f74af5ce..fbef9dc0 100644
>> --- a/drm/nouveau/nouveau_encoder.h
>> +++ b/drm/nouveau/nouveau_encoder.h
>> @@ -65,6 +65,10 @@ struct nouveau_encoder {
>>                         int link_bw;
>>                         bool no_interlace;
>>                 } dp;
>> +
>> +               struct {
>> +                       uint16_t max_mhz;
>> +               } tmds;
>>         };
>>
>>         void (*enc_save)(struct drm_encoder *encoder);
>> --
>> 2.17.1
>>
>> _______________________________________________
>> Nouveau mailing list
>> Nouveau@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/nouveau
On 03/08/18 16:56, Karol Herbst wrote:
> On Fri, Aug 3, 2018 at 4:08 PM, Ilia Mirkin <imirkin@alum.mit.edu> wrote:
>> On Fri, Aug 3, 2018 at 8:19 AM, Karol Herbst <kherbst@redhat.com> wrote:
>>> v2: clean up left over comments
>>>      don't overwrite hdmimhz parameter
>>>      cap to 297MHz
>>>
>>> Signed-off-by: Karol Herbst <kherbst@redhat.com>
>>> ---
>>>   drm/nouveau/dispnv50/disp.c     |  5 +++++
>>>   drm/nouveau/nouveau_connector.c | 15 ++++++++++-----
>>>   drm/nouveau/nouveau_encoder.h   |  4 ++++
>>>   3 files changed, 19 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drm/nouveau/dispnv50/disp.c b/drm/nouveau/dispnv50/disp.c
>>> index fa23d7a2..103433cb 100644
>>> --- a/drm/nouveau/dispnv50/disp.c
>>> +++ b/drm/nouveau/dispnv50/disp.c
>>> @@ -1433,7 +1433,12 @@ nv50_sor_create(struct drm_connector *connector, struct dcb_output *dcbe,
>>>          case DCB_OUTPUT_LVDS: type = DRM_MODE_ENCODER_LVDS; break;
>>>          case DCB_OUTPUT_DP:
>>>                  nv_encoder->dp.no_interlace = caps->sor[or].dp.no_interlace;
>>> +               type = DRM_MODE_ENCODER_TMDS;
>>> +               break;
>>>          case DCB_OUTPUT_TMDS:
>>> +               nv_encoder->tmds.max_mhz = caps->sor[or].tmds.max_mhz;
>>> +               type = DRM_MODE_ENCODER_TMDS;
>>> +               break;
>>>          default:
>>>                  type = DRM_MODE_ENCODER_TMDS;
>>>                  break;
>>> diff --git a/drm/nouveau/nouveau_connector.c b/drm/nouveau/nouveau_connector.c
>>> index 074e6d52..65fac604 100644
>>> --- a/drm/nouveau/nouveau_connector.c
>>> +++ b/drm/nouveau/nouveau_connector.c
>>> @@ -980,15 +980,20 @@ static unsigned
>>>   get_tmds_link_bandwidth(struct drm_connector *connector, bool hdmi)
>>>   {
>>>          struct nouveau_connector *nv_connector = nouveau_connector(connector);
>>> +       struct nouveau_encoder *nv_encoder = nv_connector->detected_encoder;
>>>          struct nouveau_drm *drm = nouveau_drm(connector->dev);
>>>          struct dcb_output *dcb = nv_connector->detected_encoder->dcb;
>>>
>>> +       if (hdmi && nouveau_hdmimhz > 0)
>>> +               return nouveau_hdmimhz * 1000;
>>> +
>>> +       if (nv_encoder->tmds.max_mhz)
>>> +               /* no HDMI 2.0 for now and not quite clear if we can use
>>> +                 * higher HDMI clocks than 297MHz
>>> +                 */
>>> +               return min(297, nv_encoder->tmds.max_mhz) * 1000;
>> So ... will you start reporting 297MHz single-link TMDS bandwidth over DVI now?
>>
>> What values show up in max_mhz in practice, across the boards you've
>> tested (ideally a per-generation summary would be nice).
>>
> It is a bit silly that those limits are multiple of 10MHz though :/
>
> Fermi (GF119, allthough those caps are there starting with GF110, but
> changing the code for that is a bit more messy currently): 230
> Kepler: 340
> Maxwell+: 600

Do you have ideas on whether the limit shouldn't rather be the minimum 
of 1) the CAPS-indicated limit and 2) the pixel clock PLL limit derived 
from the PLL limits table (and/or other related VBIOS tables)? Is there 
a reason to believe NVIDIA deliberately hard-coded an upper bound of 
297MHz? Can you perhaps try really hard to nudge the official driver to 
set pixel clocks on Kepler somewhere between 297 and 340MHz by messing 
with the VBIOS tables and modelines (even if it fails to bring the 
display up because the PLL never locks or sth...)?
>
> I really want to test this 340MHz limit on Kepler before though.
>
> we also have a few more caps (which are unrelated to the max clock
> over DVI, but maybe you have some ideas where we could use them or fix
> potential issues we don't know yet how to fix them?):
> SINGLE_LVDS18
> SINGLE_LVDS24
> DUAL_LVDS18
> DUAL_LVDS24
> SINGLE_TMDS_A
> SINGLE_TMDS_B
> DUAL_TMDS
> DP_A
> DP_B
>
> besides that there is tons of stuff inside
> open-gpu-doc/Display-Class-Methods/2/cl907d.h, it is the
> CORE_NOTIFIER_3_CAPABILITIES stuff.
>
>>> +
>>>          if (hdmi) {
>>> -               if (nouveau_hdmimhz > 0)
>>> -                       return nouveau_hdmimhz * 1000;
>>> -               /* Note: these limits are conservative, some Fermi's
>>> -                * can do 297 MHz. Unclear how this can be determined.
>>> -                */
>>>                  if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_KEPLER)
>>>                          return 297000;
>>>                  if (drm->client.device.info.family >= NV_DEVICE_INFO_V0_FERMI)
>>> diff --git a/drm/nouveau/nouveau_encoder.h b/drm/nouveau/nouveau_encoder.h
>>> index f74af5ce..fbef9dc0 100644
>>> --- a/drm/nouveau/nouveau_encoder.h
>>> +++ b/drm/nouveau/nouveau_encoder.h
>>> @@ -65,6 +65,10 @@ struct nouveau_encoder {
>>>                          int link_bw;
>>>                          bool no_interlace;
>>>                  } dp;
>>> +
>>> +               struct {
>>> +                       uint16_t max_mhz;
>>> +               } tmds;
>>>          };
>>>
>>>          void (*enc_save)(struct drm_encoder *encoder);
>>> --
>>> 2.17.1
>>>
>>> _______________________________________________
>>> Nouveau mailing list
>>> Nouveau@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
> _______________________________________________
> Nouveau mailing list
> Nouveau@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau