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 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7EEDBC433EF for ; Thu, 14 Oct 2021 20:21:35 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3B56B610D2 for ; Thu, 14 Oct 2021 20:21:35 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 3B56B610D2 Authentication-Results: mail.kernel.org; dmarc=fail (p=quarantine dis=none) header.from=amd.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D7E676EC3A; Thu, 14 Oct 2021 20:21:33 +0000 (UTC) Received: from NAM10-BN7-obe.outbound.protection.outlook.com (mail-bn7nam10on2056.outbound.protection.outlook.com [40.107.92.56]) by gabe.freedesktop.org (Postfix) with ESMTPS id AF5D16EC3A for ; Thu, 14 Oct 2021 20:21:31 +0000 (UTC) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KDBWNSVcBUOIimv5Cwg8qWW9J5ADfkr4mANUGEeoDlv47Whd0uPMukX7nmEi+N7WBIN01DQwgFAraoPlEwBO7FfjEnDyxtgfGOxIzH98iQPCBWUGpz86eC8Otm95s0vXtt0Pjljb5FBKK1IiSkFHJHilat0a904dFOW0yvtmHd6l9jsivisJsgbM5zpNQatfFV9pt5CmN+gbi1kMvWPh5+EMMqzHxW5e4SdtbGRJpP8N6HC1GpmWunI05A3piKK6ycyTSFfISc+qKuuNAaVgSEJELGdc5cJ9pyWlhXOCFRq3zM8a9QWCZO/hwbZgY/q86560oed2A44PI4H7nKTO0A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ZCi+5Xr6mvzHp9B3WR4Z/WM04gpya5oK1A1sE0189xE=; b=jH0Fw9dxRV9/rCNaKhErMzvQcKF0zyQcREnNoA6KHJoL7Zym4gul1sw1sjxBAn6r7xDqMDKSDZ1IMum8p1QNZ1vrSzWpEhDxduhTXa7yh2Tl8amBtb5QJi3D9GHCJ06Wvv43zjJvZAZgUDrq0OhZgGkufqKKrB6tsuvZO7mocRNuaMn4CtIIu1PCZQHGfLX7x13hZdZILMzUI/AgEFViuaHjA9/mnRKNbxQvywevP5LEYf42H1Tz4ZEua6urlvmaQDI0GC8q3BzMsENkr9chdZWF352XdOZit716VOh3NCku5NxrOW7RsfzZZCC6FJ6UMHObMJwG5g6W5CYGp5fWRw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ZCi+5Xr6mvzHp9B3WR4Z/WM04gpya5oK1A1sE0189xE=; b=S8S18VG3i59wkurs7HGWpbI/Rv7JiZMDw8yXgrY/3PwJt6XrPBjkcMEYsRQSi1eGTtMM3MaNCyCJLPJ1IETm45WUx+menpWxVJEQi+sz2ZWk7P/X1f5anjsdkpDM/gFhaF0PD9y3VqU6nGyU2fy9+2UemKTxI7Cpu8+BvXvgvdw= Authentication-Results: amd.com; dkim=none (message not signed) header.d=none;amd.com; dmarc=none action=none header.from=amd.com; Received: from MW3PR12MB4554.namprd12.prod.outlook.com (2603:10b6:303:55::21) by MW3PR12MB4538.namprd12.prod.outlook.com (2603:10b6:303:55::15) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4587.19; Thu, 14 Oct 2021 20:21:28 +0000 Received: from MW3PR12MB4554.namprd12.prod.outlook.com ([fe80::700d:46f0:61ab:9c1c]) by MW3PR12MB4554.namprd12.prod.outlook.com ([fe80::700d:46f0:61ab:9c1c%9]) with mapi id 15.20.4608.016; Thu, 14 Oct 2021 20:21:28 +0000 Subject: Re: [PATCH] drm: Update MST First Link Slot Information Based on Encoding Format To: Lyude Paul , Jerry.Zuo@amd.com, dri-devel@lists.freedesktop.org Cc: Harry.Wentland@amd.com, Wayne.Lin@amd.com, Nicholas.Kazlauskas@amd.com References: <5e463fbc3d633eea1078e838ba5be0065ffbeb1e.camel@redhat.com> <20211012215848.1507023-1-Bhawanpreet.Lakha@amd.com> <3fbf786ee687e57cab02d71c745d01fb39819cba.camel@redhat.com> From: Bhawanpreet Lakha Message-ID: Date: Thu, 14 Oct 2021 16:21:24 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 In-Reply-To: <3fbf786ee687e57cab02d71c745d01fb39819cba.camel@redhat.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-ClientProxiedBy: YT1PR01CA0123.CANPRD01.PROD.OUTLOOK.COM (2603:10b6:b01:2c::32) To MW3PR12MB4554.namprd12.prod.outlook.com (2603:10b6:303:55::21) MIME-Version: 1.0 Received: from [IPv6:2607:fea8:9dd:3f70::28d8] (2607:fea8:9dd:3f70::28d8) by YT1PR01CA0123.CANPRD01.PROD.OUTLOOK.COM (2603:10b6:b01:2c::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4608.15 via Frontend Transport; Thu, 14 Oct 2021 20:21:26 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 09180522-9ad3-4adc-c483-08d98f5033b3 X-MS-TrafficTypeDiagnostic: MW3PR12MB4538: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:5516; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: q7YPbr2rE8G3cf9UYeV6mD79L0Tr0GO7tJLCRSQca4n6h6NdYVkcc2A5Kqk5pq7tS1uJu+HVM6HBJSqSx8zWQZdL2EwRAbY9xJd0KfksULOYWJHbgJgXQniveD5c/FXWZCVWT0iebolrLF9jPSq+K+qnASqu/LRDXVefaB+eSxHPoHbFHGj6vHxxdv3VRiwRnel8WVGGesZW6hCvSbGOomC1T518/CSsKd2bnTFY2LjnuuZTLoQJBSR/nbCs6pJqRdyqVxEEzZMik4LWKk2Zxga5GnUhuIFudhfFPXEyggdervHbXK2TFaDtGqZ1XKZMvm8s49F6pmmFt8wEAujxiovVIthTNBN4BmkRAXH8D8RfaQSmm4PAmI2pEo1JIy0lZzupb3kJKaC8fTVzrB5eGQKCZATx0cDhIDkIpm88cjBFxjzqxhdZm20JAOV/k3qM5favvjnLY7sYFTWll6Yn78kELlO167v2Uw8n4UtFZXc0mOV+v9fkXQlPO+Y+bBqyjqZrcE5iyk/BoPC0QDGwSzD3wmzT+RIkr5PdLuyfJkjRO0BqtLS7qWbMGGQ6r3mtv5q5zOODrxdklBHuo/grxOWGs3LuvMw5yo6LY/bdHE2KM9Cf+n8YcBcioV1u6QxRY8hcUl7p8JmZjmQa2mRyrlvX8w5OgPqcigN1JO/lrC82UFHM0Wb6aOObNpiBJUXle1AkQcOWaOXp30ZaHKpOANcjA9XDSh4WYYVnEos/UnQ= X-Forefront-Antispam-Report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MW3PR12MB4554.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(86362001)(66946007)(4001150100001)(30864003)(15650500001)(8676002)(316002)(186003)(6486002)(66556008)(66476007)(2906002)(4326008)(83380400001)(8936002)(53546011)(5660300002)(38100700002)(2616005)(508600001)(36756003)(31686004)(31696002)(43740500002)(45980500001); DIR:OUT; SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cWR4WGxudm8xd2pNR3BaL0NrY2MvNDgzWHNzcDE1M2hUZWRDM0pzYUNvT09Y?= =?utf-8?B?WEE5NlAwZDBCVVZXTG1SaG9XSERuMkhtNFY4Wm9oRm90Zk4vcUhHdjVsUHdS?= =?utf-8?B?WDFrWmdZRFZnSFp0ODExa2NmakovVlR4a3c1RDJoSDg2TFdQaUw3WVJic0VB?= =?utf-8?B?YTFzNGhBNVU1aW5yRWFscWZibm9sQzNaZWpHTlptdE9pYXlabHZIR2loNFdl?= =?utf-8?B?WFhvdmJNd3hFNUZ1MjFBMVVhRlJidHlnL1ZXblQzTE90N2hGbUtlOG9TVEpK?= =?utf-8?B?SzRJTGJzWEJSQk02WEU2ZzczZDNqbnNpRmhZTm1XVU4yR3FYSXVldnZ2VGY3?= =?utf-8?B?eFVYZ05ydk0xS2trWVFZQ0t2cERkVC94TFhpakhybFZwVFRNcExRRTR0MWNl?= =?utf-8?B?RzdZemRuVFM3WDc2cmRtOEJHVjlXelY0aXpPMW1DNkdwZW92cEpDcDZ3MUl2?= =?utf-8?B?WUNUYm53WFB5aHNBZU1YaG5oanVLRVNTV2JuaG5YbEVRRGhZVStFV3ExTnky?= =?utf-8?B?VWxZM1p3WTVDV3N6cWNDbFhlRXlaeTdpYmFGSXIvd1hNU3VWZ1ZaejFxSjJM?= =?utf-8?B?dUJoYzhKZGtvS0lZQWU3TytFVyt6MDA3Mmh2SmVDM2NleWxQQW1SelRLMkpH?= =?utf-8?B?VkowYktlSHlNdXU4RXVJekJWNEo5bkdqL1BKdWZ6SG1LSUNKY0dVQkZxZDdz?= =?utf-8?B?Wk1CTDZjUUszUHYrVWcrNitvVEE2aEVxTmU4K2gvUm56aVQ4SGpsc0t4R0h6?= =?utf-8?B?dzF0blQzR09DeTRITE1JMVUzUFA0NWIzRlFCS1JWWmphUXZpcUk1UU9GUExu?= =?utf-8?B?ell4S2xuY1pwdjNiZ2djV1k1MmVMVm1BSkgzcW1CMTlLTVo0TlIwTWZZT29m?= =?utf-8?B?c0pHS3V6SU9iK0MvY2FudjJ1bHd0RllMVTAzYWp2elVQRjNMdjNHTTY0cVJq?= =?utf-8?B?aUV3b2REUzlXRytuWWxxNFEwaENIT2hNTkoxeGZMRVJUeXNpTk90aHBIMXEw?= =?utf-8?B?SEJOZzZhNlc1S2puWmdIUkhHNGlUd05rWm9ma2pndk9BT3dDT0d1aDJ1ajdE?= =?utf-8?B?TmczYXpvWlhnVjVuSzQwdnVrbWV2cU5HdnVnQ1E1YkVQUGxuWEdYWEhROEhu?= =?utf-8?B?azVub211N3gxcmpQaTBENENlRlJlNFBmcmhVcW1GaTRETkRYYzhDRVJhYWxC?= =?utf-8?B?S2pabUdWUnJhS3R2SSs4RkNCNk5IRTdSMVVOOWV6YjZRWFlOZ29FcmhBNmdw?= =?utf-8?B?dUpiRlJnaEppK2pjRXhoZ0N5V0Y1T3loTE1STG9PRHRNeUtxNVhjYzJyM2VF?= =?utf-8?B?Y3VuR3lLNUdkeUI0VTZ1dFFjMjJwb1NralkrUTlOT2xIMGlpYzJLZzhycmJ6?= =?utf-8?B?aXdINFFuRzFhdE1WeEs5a0g3NGRUTVlmWklBZDJiVTJ0K0ZjV1h6cWJPdGsz?= =?utf-8?B?NEVWZHBxaXh0Nm5jQk1LQWJ3N0hzcVdaamRLNEtFSDREMFZDbC9oRzZIUHVj?= =?utf-8?B?YnVNQlU1U2VvWkt1SlBqcGxrdnVVcTY3YklIMFVGd0JGM3hJZGlHWXhJVmNK?= =?utf-8?B?YmhNazF1ckRLaFAzSjZNWVo3eGdvNEMySE9JMGs0TnpzNmRRQmZEdVNlVWpI?= =?utf-8?B?S0tmN3dqdldmYU51eHRMWHhlRkh6OVlSQ0kxcStPUC9RalFSQW41ZnpNM0tE?= =?utf-8?B?SXhKcDgyWklrbVZ6QUNtdCtPaUg0eVRIM253K1VybXYvNzJUYzBNbm8vZzdp?= =?utf-8?B?N3M4QVJIT1h2czJ2MTBKRUdHb2xmVjdFQUVwbjlUSnk1OGpvUmdHbjZ6Q3dQ?= =?utf-8?B?R3ZHYkJxSDJxQnZpeUtQZz09?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 09180522-9ad3-4adc-c483-08d98f5033b3 X-MS-Exchange-CrossTenant-AuthSource: MW3PR12MB4554.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Oct 2021 20:21:27.8933 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: qKjrCW8HwgdZ8ktT4Ky70zknEHRRzk1O6MlmhJIqke0wVNcHQHE5TP6LGqfW0i/V X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW3PR12MB4538 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On 2021-10-13 6:25 p.m., Lyude Paul wrote: > Some comments below (also, sorry again for the mixup on the last review!) > > On Tue, 2021-10-12 at 17:58 -0400, Bhawanpreet Lakha wrote: >> 8b/10b encoding format requires to reserve the first slot for >> recording metadata. Real data transmission starts from the second slot, >> with a total of available 63 slots available. >> >> In 128b/132b encoding format, metadata is transmitted separately >> in LLCP packet before MTP. Real data transmission starts from >> the first slot, with a total of 64 slots available. >> >> v2: >> * Remove get_mst_link_encoding_cap >> * Move total/start slots to mst_state, and copy it to mst_mgr in >> atomic_check >> >> Signed-off-by: Fangzhi Zuo >> Signed-off-by: Bhawanpreet Lakha >> --- >>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 28 +++++++++++++++ >>  drivers/gpu/drm/drm_dp_mst_topology.c         | 35 +++++++++++++++---- >>  include/drm/drm_dp_mst_helper.h               | 13 +++++++ >>  3 files changed, 69 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> index 5020f2d36fe1..4ad50eb0091a 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -10612,6 +10612,8 @@ static int amdgpu_dm_atomic_check(struct drm_device >> *dev, >>  #if defined(CONFIG_DRM_AMD_DC_DCN) >>         struct dsc_mst_fairness_vars vars[MAX_PIPES]; >>  #endif >> +       struct drm_dp_mst_topology_state *mst_state; >> +       struct drm_dp_mst_topology_mgr *mgr; >> >>         trace_amdgpu_dm_atomic_check_begin(state); >> >> @@ -10819,6 +10821,32 @@ static int amdgpu_dm_atomic_check(struct drm_device >> *dev, >>                 lock_and_validation_needed = true; >>         } >> >> +#if defined(CONFIG_DRM_AMD_DC_DCN) >> +       for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { >> +               struct amdgpu_dm_connector *aconnector; >> +               struct drm_connector *connector; >> +               struct drm_connector_list_iter iter; >> +               u8 link_coding_cap; >> + >> +               if (!mgr->mst_state ) >> +                       continue; > extraneous space > >> + >> +               drm_connector_list_iter_begin(dev, &iter); >> +               drm_for_each_connector_iter(connector, &iter) { >> +                       int id = connector->index; >> + >> +                       if (id == mst_state->mgr->conn_base_id) { >> +                               aconnector = >> to_amdgpu_dm_connector(connector); >> +                               link_coding_cap = >> dc_link_dp_mst_decide_link_encoding_format(aconnector->dc_link); >> +                               drm_dp_mst_update_coding_cap(mst_state, >> link_coding_cap); >> + >> +                               break; >> +                       } >> +               } >> +               drm_connector_list_iter_end(&iter); >> + >> +       } >> +#endif >>         /** >>          * Streams and planes are reset when there are changes that affect >>          * bandwidth. Anything that affects bandwidth needs to go through >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c >> b/drivers/gpu/drm/drm_dp_mst_topology.c >> index ad0795afc21c..fb5c47c4cb2e 100644 >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c >> @@ -3368,7 +3368,7 @@ int drm_dp_update_payload_part1(struct >> drm_dp_mst_topology_mgr *mgr) >>         struct drm_dp_payload req_payload; >>         struct drm_dp_mst_port *port; >>         int i, j; >> -       int cur_slots = 1; >> +       int cur_slots = mgr->start_slot; >>         bool skip; >> >>         mutex_lock(&mgr->payload_lock); >> @@ -4321,7 +4321,7 @@ int drm_dp_find_vcpi_slots(struct >> drm_dp_mst_topology_mgr *mgr, >>         num_slots = DIV_ROUND_UP(pbn, mgr->pbn_div); >> >>         /* max. time slots - one slot for MTP header */ >> -       if (num_slots > 63) >> +       if (num_slots > mgr->total_avail_slots) >>                 return -ENOSPC; > For reasons I will explain a little further in this email, we might want to > drop this… > >>         return num_slots; >>  } >> @@ -4333,7 +4333,7 @@ static int drm_dp_init_vcpi(struct >> drm_dp_mst_topology_mgr *mgr, >>         int ret; >> >>         /* max. time slots - one slot for MTP header */ >> -       if (slots > 63) >> +       if (slots > mgr->total_avail_slots) > …and this > >>                 return -ENOSPC; >> >>         vcpi->pbn = pbn; >> @@ -4507,6 +4507,18 @@ int drm_dp_atomic_release_vcpi_slots(struct >> drm_atomic_state *state, >>  } >>  EXPORT_SYMBOL(drm_dp_atomic_release_vcpi_slots); >> >> +void drm_dp_mst_update_coding_cap(struct drm_dp_mst_topology_state >> *mst_state, uint8_t link_coding_cap) > Need some kdocs here > >> +{ >> +       if (link_coding_cap == DP_CAP_ANSI_128B132B) { >> +               mst_state->total_avail_slots = 64; >> +               mst_state->start_slot = 0; >> +       } >> + >> +       DRM_DEBUG_KMS("%s coding format on mgr 0x%p\n", >> +                       (link_coding_cap == DP_CAP_ANSI_128B132B) ? >> "128b/132b":"8b/10b", mst_state->mgr); > need to fix indenting here, and wrap this to 100 chars > >> +} >> +EXPORT_SYMBOL(drm_dp_mst_update_coding_cap); >> + >>  /** >>   * drm_dp_mst_allocate_vcpi() - Allocate a virtual channel >>   * @mgr: manager for this port >> @@ -4538,8 +4550,8 @@ bool drm_dp_mst_allocate_vcpi(struct >> drm_dp_mst_topology_mgr *mgr, >> >>         ret = drm_dp_init_vcpi(mgr, &port->vcpi, pbn, slots); >>         if (ret) { >> -               drm_dbg_kms(mgr->dev, "failed to init vcpi slots=%d max=63 >> ret=%d\n", >> -                           DIV_ROUND_UP(pbn, mgr->pbn_div), ret); >> +               drm_dbg_kms(mgr->dev, "failed to init vcpi slots=%d max=%d >> ret=%d\n", >> +                           DIV_ROUND_UP(pbn, mgr->pbn_div), mgr- >>> total_avail_slots, ret); >>                 drm_dp_mst_topology_put_port(port); >>                 goto out; >>         } >> @@ -5226,7 +5238,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct >> drm_dp_mst_topology_mgr *mgr, >>                                          struct drm_dp_mst_topology_state >> *mst_state) >>  { >>         struct drm_dp_vcpi_allocation *vcpi; >> -       int avail_slots = 63, payload_count = 0; >> +       int avail_slots = mgr->total_avail_slots, payload_count = 0; >> >>         list_for_each_entry(vcpi, &mst_state->vcpis, next) { >>                 /* Releasing VCPI is always OK-even if the port is gone */ >> @@ -5255,7 +5267,7 @@ drm_dp_mst_atomic_check_vcpi_alloc_limit(struct >> drm_dp_mst_topology_mgr *mgr, >>                 } >>         } >>         drm_dbg_atomic(mgr->dev, "[MST MGR:%p] mst state %p VCPI avail=%d >> used=%d\n", >> -                      mgr, mst_state, avail_slots, 63 - avail_slots); >> +                      mgr, mst_state, avail_slots, mgr->total_avail_slots - >> avail_slots); >> >>         return 0; >>  } >> @@ -5421,6 +5433,10 @@ int drm_dp_mst_atomic_check(struct drm_atomic_state >> *state) >>                         break; >> >>                 mutex_lock(&mgr->lock); >> + >> +               mgr->start_slot = mst_state->start_slot; >> +               mgr->total_avail_slots = mst_state->total_avail_slots; >> + > this isn't correct - atomic checks aren't allowed to mutate anything besides > the atomic state structure that we're checking since we don't know whether or > not the display state is going to be committed when checking it. If we're > storing state in mgr, that state needs to be written to mgr during the atomic > commit instead of the atomic check. > > ...but, coming back to this MST code after being gone for a while, I think it > might be time for us to stop worrying about the non-atomic state. Especially > since there's only one driver using it; the legacy radeon.ko; and right now > the plan is either to just drop MST support on radeon.ko or move the MST code > it uses into radeon.ko.Which brings me to say - I think we can drop some of > the hunks I mentioned above (e.g. the changes to drm_dp_init_vcpi and > drm_dp_find_vcpi_slots I mentioned above). We can then just update the kdocs > for these functions in a separate patch to clarify that now in addition to > being deprecated, these functions are just broken and will eventually be > removed. > > So - doing that allows us to get rid of mgr->total_avail_slots and mgr- >> start_slot entirely, just set the slot info in the atomic state during atomic > check, and then just rely on the atomic state for > drm_dp_atomic_find_vcpi_slots() and friends. Which seems much simpler to me, > does this sound alrght with you? Thanks for the response, That function is per port (drm_dp_atomic_find_vcpi_slots) so not sure how that will work, maybe I don't understand what you mean? Also we only need to check this inside drm_dp_mst_atomic_check_vcpi_alloc_limit(), which doesn't have a state, so we still need to have this on the mgr somehow. I was thinking we could add the slots(or some DP version indicator) inside the drm_connector, and add a parameter to drm_dp_mst_atomic_check_vcpi_alloc_limit(int slots) and call it with this info via drm_dp_mst_atomic_check(). Bhawan > >>                 ret = drm_dp_mst_atomic_check_mstb_bw_limit(mgr- >>> mst_primary, >>                                                             mst_state); >>                 mutex_unlock(&mgr->lock); >> @@ -5527,11 +5543,16 @@ int drm_dp_mst_topology_mgr_init(struct >> drm_dp_mst_topology_mgr *mgr, >>         if (!mgr->proposed_vcpis) >>                 return -ENOMEM; >>         set_bit(0, &mgr->payload_mask); >> +       mgr->total_avail_slots = 63; >> +       mgr->start_slot = 1; >> >>         mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL); >>         if (mst_state == NULL) >>                 return -ENOMEM; >> >> +       mst_state->total_avail_slots = 63; >> +       mst_state->start_slot = 1; >> + >>         mst_state->mgr = mgr; >>         INIT_LIST_HEAD(&mst_state->vcpis); >> >> diff --git a/include/drm/drm_dp_mst_helper.h >> b/include/drm/drm_dp_mst_helper.h >> index ddb9231d0309..f8152dfb34ed 100644 >> --- a/include/drm/drm_dp_mst_helper.h >> +++ b/include/drm/drm_dp_mst_helper.h >> @@ -554,6 +554,8 @@ struct drm_dp_mst_topology_state { >>         struct drm_private_state base; >>         struct list_head vcpis; >>         struct drm_dp_mst_topology_mgr *mgr; >> +       u8 total_avail_slots; >> +       u8 start_slot; >>  }; >> >>  #define to_dp_mst_topology_mgr(x) container_of(x, struct >> drm_dp_mst_topology_mgr, base) >> @@ -661,6 +663,16 @@ struct drm_dp_mst_topology_mgr { >>          */ >>         int pbn_div; >> >> +       /** >> +        * @total_avail_slots: 63 for 8b/10b, 64 for 128/132b >> +        */ >> +       u8 total_avail_slots; >> + >> +       /** >> +        * @start_slot: 1 for 8b/10b, 0 for 128/132b >> +        */ >> +       u8 start_slot; >> + >>         /** >>          * @funcs: Atomic helper callbacks >>          */ >> @@ -806,6 +818,7 @@ int drm_dp_mst_get_vcpi_slots(struct >> drm_dp_mst_topology_mgr *mgr, struct drm_dp >> >>  void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr, >> struct drm_dp_mst_port *port); >> >> +void drm_dp_mst_update_coding_cap(struct drm_dp_mst_topology_state >> *mst_state, uint8_t link_coding_cap); >> >>  void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, >>                                 struct drm_dp_mst_port *port);