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

[Roy Spliet]

On 03/08/18 16:56, Karol Herbst wrote:
> On Fri, Aug 3, 2018 at 4:08 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>> On Fri, Aug 3, 2018 at 8:19 AM, Karol Herbst <kherbst at redhat.com> wrote:
>>> v2: clean up left over comments
>>>      don't overwrite hdmimhz parameter
>>>      cap to 297MHz
>>>
>>> Signed-off-by: Karol Herbst <kherbst at 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 at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/nouveau
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau