From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.1 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 99839C4727F for ; Fri, 25 Sep 2020 22:08:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 4FA542076D for ; Fri, 25 Sep 2020 22:08:14 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="LLshTXEv" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728051AbgIYWIN (ORCPT ); Fri, 25 Sep 2020 18:08:13 -0400 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:37652 "EHLO us-smtp-delivery-124.mimecast.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726731AbgIYWIN (ORCPT ); Fri, 25 Sep 2020 18:08:13 -0400 Dkim-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1601071691; h=from:from:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=oyEiYOKdzKVlvL5D6nWRiuLWCYpicqvgUJmqZ1lXRwE=; b=LLshTXEv9LEIjfKtkttp80PFnVjeDMOt2VleST9nPiowT/EnOh6WJTWwNPzCGh/FqD23n9 8PSb7nyvP/JqdS81foFqN5oduUDIFrsPmYoI1lIF18AB0WlGl+4X/gnCZnqjx9+rsDcL6+ UvaLUAqtvNcdcYLUz9gKsmwzimgcOW0= Received: from mail-qv1-f70.google.com (mail-qv1-f70.google.com [209.85.219.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-264-H5J6LlwWNpWn-e1Dd1PsNw-1; Fri, 25 Sep 2020 18:08:09 -0400 X-MC-Unique: H5J6LlwWNpWn-e1Dd1PsNw-1 Received: by mail-qv1-f70.google.com with SMTP id 99so2748604qva.1 for ; Fri, 25 Sep 2020 15:08:09 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:reply-to:to:cc:date :in-reply-to:references:organization:user-agent:mime-version :content-transfer-encoding; bh=oyEiYOKdzKVlvL5D6nWRiuLWCYpicqvgUJmqZ1lXRwE=; b=C3kQ75IfK4pCP42AEcI37F11VsFhrxUY4JIMsItgsUm6i5c+WXzjD7AlVrpkNcmeoX U3x/U39vMma69knH9tbswBrDVz0tYhXN9fLmaRGbtkWzYreoP138KRWsnVLm1f3VhvrH xFVDpH/KWNM+UpsJL6L8Qb+FqOw5LGUygHMuQ8thg/IMiBz8ERCpYLfO3QOAUQ+3fKQ3 O4l7My5Bt+gK9NR8QWcAPhTCm0twODjmrWdRS9XgzezK/FsBNIkR4L039ojIlPNbKuU/ aum1VtiWT5rvBc70XTEdVraPUNrsf/UjValml9r9hFrX34k2/7qbxvvRvlh2yAErWH6w AOvg== X-Gm-Message-State: AOAM531tEmWN2u+k7MeFaTv+zuw/zPugzZ+RKtqGfRNntdJr1QHZluNv bvZ6gt6cKuILc5dhNX3bvjRCrlpW1HnJqpgX4AL3Buw2yjEQkXc+W8w79uxL7Y2HTvvkV0O+6cZ eEk4p6PjU/TDKNhFyJJTWAICX X-Received: by 2002:a37:6805:: with SMTP id d5mr2172649qkc.116.1601071688736; Fri, 25 Sep 2020 15:08:08 -0700 (PDT) X-Google-Smtp-Source: ABdhPJyaqMI5NTbDpcpde/1VdVcXIUjX7In3tEFt7Qxf4XnALVe2p6YGM2Pd1kZuXyUxB7ftE3XQxQ== X-Received: by 2002:a37:6805:: with SMTP id d5mr2172612qkc.116.1601071688377; Fri, 25 Sep 2020 15:08:08 -0700 (PDT) Received: from Whitewolf.lyude.net (pool-108-49-102-102.bstnma.fios.verizon.net. [108.49.102.102]) by smtp.gmail.com with ESMTPSA id h68sm2659867qkf.30.2020.09.25.15.08.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 25 Sep 2020 15:08:07 -0700 (PDT) Message-ID: <8bd8ee03f88e7e674e0ea8c6d63d783777cfe414.camel@redhat.com> Subject: Re: [PATCH] drm/nouveau/kms/nv50-: Fix clock checking algorithm in nv50_dp_mode_valid() From: Lyude Paul Reply-To: lyude@redhat.com To: Ilia Mirkin Cc: nouveau , David Airlie , open list , "open list:DRM DRIVER FOR NVIDIA GEFORCE/QUADRO GPUS" , Ben Skeggs Date: Fri, 25 Sep 2020 18:08:06 -0400 In-Reply-To: References: <20200922210510.156220-1-lyude@redhat.com> <7b10668ee337e531b14705ebecb1f6c1004728d6.camel@redhat.com> Organization: Red Hat Content-Type: text/plain; charset="UTF-8" User-Agent: Evolution 3.36.5 (3.36.5-1.fc32) MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 2020-09-22 at 17:22 -0400, Ilia Mirkin wrote: > On Tue, Sep 22, 2020 at 5:14 PM Lyude Paul wrote: > > On Tue, 2020-09-22 at 17:10 -0400, Ilia Mirkin wrote: > > > Can we use 6bpc on arbitrary DP monitors, or is there a capability for > > > it? Maybe only use 6bpc if display_info.bpc == 6 and otherwise use 8? > > > > I don't think that display_info.bpc actually implies a minimum bpc, only a > > maximum bpc iirc (Ville would know the answer to this one). The other thing > > to > > note here is that we want to assume the lowest possible bpc here since we're > > only concerned if the mode passed to ->mode_valid can be set under -any- > > conditions (including those that require lowering the bpc beyond it's > > maximum > > value), so we definitely do want to always use 6bpc here even once we get > > support for optimizing the bpc based on the available display bandwidth. > > Yeah, display_info is the max bpc. But would an average monitor > support 6bpc? And if it does, does the current link training code even > try that when display_info.bpc != 6? So I did confirm that 6bpc support is mandatory for DP, so yes-6 bpc will always work. But also, your second comment doesn't really apply here. So: to be clear, we're not really concerned here about whether nouveau will actually use 6bpc or not. In truth I'm not actually sure either if we have any code that uses 6bpc (iirc we don't), since we don't current optimize bpc. I think it's very possible for us to use 6bpc for eDP displays if I recall though, but I'm not sure on that. But that's also not the point of this code. ->mode_valid() is only used in two situations in DRM modesetting: when probing connector modes, and when checking if a mode is valid or not during the atomic check for atomic modesetting. Its purpose is only to reject display modes that are physically impossible to set in hardware due to static hardware constraints. Put another way, we only check the given mode against constraints which will always remain constant regardless of the rest of the display state. An example of a static constraint would be the max pixel clock supported by the hardware, since on sensible hardware this never changes. A dynamic constraint would be something like how much bandwidth is currently unused on an MST topology, since that value is entirely dependent on the rest of the display state. So - with that said, bpc is technically a dynamic constraint because while a sink and source both likely have their own bpc limits, any bpc which is equal or below that limit can be used depending on what the driver decides - which will be based on the max_bpc property, and additionally for MST displays it will also depend on the available bandwidth on the topology. The only non-dynamic thing about bpc is that at a minimum, it will be 6 - so any mode that doesn't fit on the link with a bpc of 6 is guaranteed to be a mode that we'll never be able to set and therefore want to prune. So, even if we're not using 6 in the majority of situations, I'm fairly confident it's the right value here. It's also what i915 does as well (and they previously had to fix a bug that was the result of assuming a minimum of 8bpc instead of 6). > > -ilia > -- Sincerely, Lyude Paul (she/her) Software Engineer at Red Hat