Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <c88ee580-2062-1afd-dab4-967750ef26c5@nvidia.com>
Date:   Wed, 14 Jun 2023 11:39:20 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH V1] Revert "PCI: tegra194: Enable support for 256 Byte
 payload"
Content-Language: en-US
To:     Vidya Sagar <vidyas@nvidia.com>, Bjorn Helgaas <helgaas@kernel.org>
Cc:     lpieralisi@kernel.org, kw@linux.com, robh@kernel.org,
        bhelgaas@google.com, thierry.reding@gmail.com,
        Sergey.Semin@baikalelectronics.ru, linux-pci@vger.kernel.org,
        linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
        kthota@nvidia.com, mmaddireddy@nvidia.com, sagar.tv@gmail.com
References: <20230608163346.GA1204586@bhelgaas>
 <5bf550a9-146e-ad51-bbeb-c55b48478f62@nvidia.com>
From:   Jon Hunter <jonathanh@nvidia.com>
In-Reply-To: <5bf550a9-146e-ad51-bbeb-c55b48478f62@nvidia.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?eWhlTzhSa094KzIrZXpYRkhXTGRMYVljNWhmdjBwSUFzZHpmVzlJVzVhUmp6?=
 =?utf-8?B?MlYyekM3LzNXY3A2K3JNK2ZOamhZZDgzd2VudE8xNnRua1lPYlByejl3cHh1?=
 =?utf-8?B?TzJOWnRyNGpYVVUrbFNFTzVIMTR3Y29uZWNGRFRSSmVPVnRTTXloNWRaUktx?=
 =?utf-8?B?aHRKaVllaWtBRGkza2lnRHJPOUlNMzFuNERCb28yZC83REpoa0U3U2VZbExY?=
 =?utf-8?B?cHM1M2xKQUZHK1ZJOTFKSWtaUDZiRUdBdzB0K3RtYVJ6dEFmOTFMVVF6eFc3?=
 =?utf-8?B?M3IwSi84b1BDaFlKTkVaZTREQjk1eit3aWRkZjVVS2lKeEFqemxrN3VlY0ln?=
 =?utf-8?B?Z2Nqbm16RzRGMVhhbFhPT2VSakJ5c1ZnQlllS2Z3TVQ4cmkwdWJvSXhERlV0?=
 =?utf-8?B?OUkvQ21hSTVnOU9lTUh6NUN1SmxXU3VtODdXeWJWcUdCcE9HWUhQajBKTkxo?=
 =?utf-8?B?V29KOCtIVVZZdklrbU9mZ1ZnSXFJbklsTEx5aFk2eEJoSkZtOUZYaWlJcGdV?=
 =?utf-8?B?aUo4VjFIdEZjb29BVGdWWmxrZGpVV3kra09FZnU0RWxHRWwxdUlMTTJyeWZn?=
 =?utf-8?B?U2hLZVNOeWdZcjBmVFdtU0QxOGxPSWQ3UnV1NHNvS3ZhN3RlVjZCRjhmODJN?=
 =?utf-8?B?M2QxNXVxZDZlOGx5YzNnS2RKa2kySWVFeGF1VzRaeVdpZkxqSzFZMlR2cExs?=
 =?utf-8?B?YW9xVXgwN0dwd1haZ0V3SWlmSEd2c0VmV0VuaVF4Q1FZZHM4cnhub1crdnR0?=
 =?utf-8?B?d1lMaUp2YmdiNFpLNEdyU2Q1MTR6Y2pQRUwwZSszblNYM3N0Qm51R1FWSVV1?=
 =?utf-8?B?TXhmOXdFOEUzQ3h6VGhZVW1KZkxIalNwZnBqbUg0aG5yR3VJbUdyS1lsSEtQ?=
 =?utf-8?B?L3BXeitKTU0vNHhBYjd1V0p3VFZLdXRZR1EzN1N0OTVGUGlxWVJrYWU0Nzll?=
 =?utf-8?B?dmd6RjdTUnNiOHlHSzRJZklFS1ZlWGE2QVhBYkQxYll5NnkwdG1ST3lmNHRR?=
 =?utf-8?B?OWNvL0VBSjQwbjJnb3lFaVJDcVhpVGtIQ0IxMVN3WmQvbWd1K21CM3lVRDZJ?=
 =?utf-8?B?YnhGZThLMzduWlNKSGlTeHg1UGdmS0VNRFQra0xzZ0k3cm10WitrTDBIVEl4?=
 =?utf-8?B?L05wMHJ2YTR6dTcvL1MyMXNldytqYitqUERwa1REWFNSQmdKekk5emxWbXZr?=
 =?utf-8?B?NkxLVUhjWWNkRWtxWm5scE41K0NFOVdlTElmZW4xamY0RVE5UlJPenFSVnVp?=
 =?utf-8?B?VEx5KzBkN2ovLzBCU1FiZGk2dWJoR2ZNaXFna2hUbGx4QTFYdmdtVTBGY2xa?=
 =?utf-8?B?V2krQkp6VFF3WGg1MitpZklYUTlyMWhzb3ZTdEtlY0hFckNlNkp0UEoxSFdF?=
 =?utf-8?B?b2ppbVJLMG9ZSFZEWHRXTE0rRDE5aGFybWRQS2Y3UTJObFlUTDdkU1lhMytw?=
 =?utf-8?B?bUNCMVZnTVNNbUdUMHNtOHBvVUdBdng3MlVPYmxFNHREdHphd1lRTk4wK2hh?=
 =?utf-8?B?MVZDZ0pMM05PZ2dMamxrQ0UwZzZSYjVNL0NEN0h4N1BUcVN1RktPQlFrK01a?=
 =?utf-8?B?WUNSNmF1dTNieWNBSm5PSjE5RmI4UzlyZlRyUE9LV25TTFpxYWw4SlFkTDhF?=
 =?utf-8?B?NUxjUmd4M0Y3Q3dFSFE3ZCtZeG1id2tkeHI2Y05qM2Q3bzlMUGQweHk0bDFL?=
 =?utf-8?B?amZFWk5XQWZkNEdWdXZ5M1UrSHlLTlFkQUtNRGFsL2dGbTZ4djhRZVZPam9P?=
 =?utf-8?B?WUU3WmlSYWRkeVFPa2xBR3piU01sMWcrYXVrM0VtemgrcDdTcmRMMmJpejFs?=
 =?utf-8?B?TWlLWXIvMGs1S1JSbTFZNHJUNVBkdW1GY29qa0l6dVNhaHdiSDlKUXRpdk1t?=
 =?utf-8?B?WWlqSGswWE9BbGU5RFA4cW9MbmIxOElCOGQ3T0lySWdhcjZNQ2ZOQVVrUmh0?=
 =?utf-8?B?Q2R2My9VbmZ4T1VxamZlZWg2T2xpdStIaUY5NXVyd0M2bGozZHNETjhkL05W?=
 =?utf-8?B?RFoyb0RnRzE4c2hSQ1dHZ3JCZEVWL3g5Mllyb2F5dEhKQmZtQ2U0dHBGUFpU?=
 =?utf-8?B?MjR5SU1ucldMRityQmN4eEFRLzJyb1R1ay96RGthT25ERkUrUWw2ejdaNCtS?=
 =?utf-8?B?dFFLMU1KVHB6eXk4bElKODl2M1IxM1N4eXZoUUtWQ2IrNjMxNm55UVVlSmJz?=
 =?utf-8?B?ZHpGWnZUVXBVRzZ2dngycVBzVkQ5U2FoSHp5SHRabG95M1cxb2xISzJIOWky?=
 =?utf-8?B?VUF3dmJqNUVLR0RCR0ZpWVcvMHR3PT0=?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 08c30374-e926-42ba-c728-08db6cc3a0ee
X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5444.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jun 2023 10:39:28.0740
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: dlzXgin5iFkVRkUR+v5AFULgJiVjcQDcTciBxnHDOo/j+xoi4qXdtviCymIzfLBQov4/hcWrnn8XOEZOBonjJw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB8244
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED autolearn=no
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Bjorn,

On 09/06/2023 03:23, Vidya Sagar wrote:
> 
> 
> On 6/8/2023 10:03 PM, Bjorn Helgaas wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On Thu, Jun 08, 2023 at 03:06:52PM +0530, Vidya Sagar wrote:
>>> This reverts commit 4fb8e46c1bc4 ("PCI: tegra194: Enable
>>> support for 256 Byte payload")
>>>
>>> Consider a PCIe hierarchy with a PCIe switch and a device connected
>>> downstream of the switch that has support for MPS which is the minimum
>>> in the hierarchy, and root port programmed with an MPS in its DevCtl
>>> register that is greater than the minimum. In this scenario, the default
>>> bus configuration of the kernel i.e. "PCIE_BUS_DEFAULT" doesn't
>>> configure the MPS settings in the hierarchy correctly resulting in the
>>> device with support for minimum MPS in the hierarchy receiving the TLPs
>>> of size more than that. Although this can be addresed by appending
>>> "pci=pcie_bus_safe" to the kernel command line, it doesn't seem to be a
>>> good idea to always have this commandline argument even for the basic
>>> functionality to work.
>>> Reverting commit 4fb8e46c1bc4 ("PCI: tegra194: Enable support for 256
>>> Byte payload") avoids this requirement and ensures that the basic
>>> functionality of the devices irrespective of the hierarchy and the 
>>> MPS of
>>> the devices in the hierarchy.
>>> To reap the benefits of having support for higher MPS, optionally, 
>>> one can
>>> always append the kernel command line with "pci=pcie_bus_perf".
>>
>> Please add blank lines between paragraphs and wrap to fill 75 columns.
>> Also add a period at the end of the very first sentence.
>>
>> s/addresed/addressed/
>>
> I'll address your comments in the next patch.
> 
>> I guess that without 4fb8e46c1bc4, Linux configured everything with
>> 128 byte MPS, and 4fb8e46c1bc4 was intended as an optimization to
>> allow 256 byte MPS.
> Correct.
> 
>>
>> If the Root Port advertises Max_Payload_Size Supported as 256 bytes in
>> DevCap, and the PCI core doesn't configure MPS=256 when possible, I'd
>> argue that should be fixed in the PCI core without a driver change
>> like this.
> Well, kernel does configure MPS=256 but only if the 'perf' configuration
> option is selected. 'perf' configuration option also changes the MRRS,
> to extract the maximum performance. I'm not sure about the reasons for 
> not making 'perf' configuration as the default configuration though.
> (IIUC, this is what you are coming to, right?)
> 
> The current patch which is a revert of an earlier patch is to keep 
> things working for different PCIe hierarchies given the default
> configuration that kernel is using at the moment.


Any more feedback on this? If not, would be great to queue for v6.5. 
Feel free to add my ...

Acked-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

-- 
nvpublic