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 41E93C433EF for ; Sat, 16 Oct 2021 10:16:03 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 19F62611AF for ; Sat, 16 Oct 2021 10:16:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S240042AbhJPKSJ (ORCPT ); Sat, 16 Oct 2021 06:18:09 -0400 Received: from ip-15.mailobj.net ([213.182.54.15]:39542 "EHLO msg-4.mailo.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235335AbhJPKSI (ORCPT ); Sat, 16 Oct 2021 06:18:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=net-c.es; s=mailo; t=1634379354; bh=zqWX1gJTKZ5q18uqa5c3Zf7qkP/A+CGNlGSRL6MHo7E=; h=X-EA-Auth:Date:From:To:Cc:Subject:Message-ID:References: MIME-Version:Content-Type:In-Reply-To; b=sMf3huKcgBE4f9Qx46mnqxxW0kG/4lohTFqKS0NtZxNeWRKEMyEaEtphj4CMJ+lgy /yujIldahtXxN2v73vppnv9iR+b7D3zue7w+NHiEgHvg8SjPPoR/slscBcmeAG2zbg QcfsijBw/h/KFQfk1ZZF+RMG9rE7rwfK3oj+PkBY= Received: by b-3.in.mailobj.net [192.168.90.13] with ESMTP via ip-206.mailobj.net [213.182.55.206] Sat, 16 Oct 2021 12:15:54 +0200 (CEST) X-EA-Auth: yKCNRkwyeIQN4zIZhVP/t5GZ4LXBCXpTe707pgk8CcqdT5Z8+L4zajcnVM6jjzg+UKc0YJNBMB/9WGFbSHtL4/w+9RPySOhn Date: Sat, 16 Oct 2021 12:15:51 +0200 From: Claudio Suarez To: Harry Wentland Cc: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-tegra@vger.kernel.org, intel-gfx@lists.freedesktop.org, David Airlie , Daniel Vetter , Laurent Pinchart , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Alex Deucher , Christian =?iso-8859-1?Q?K=F6nig?= , Pan Xinhui , Emma Anholt , Maxime Ripard , Thierry Reding , Patrik Jakobsson , Jingoo Han , Rob Clark , Sean Paul , linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, Chen-Yu Tsai , Sandy Huang , heiko@sntech.de, Neil Armstrong , Robert Foss , Ben Skeggs , nouveau@lists.freedesktop.org Subject: Re: [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver Message-ID: References: <20211015113713.630119-1-cssk@net-c.es> <20211015113713.630119-3-cssk@net-c.es> <62a3ee8d-9439-9275-4e71-876b865b9a7d@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <62a3ee8d-9439-9275-4e71-876b865b9a7d@amd.com> Precedence: bulk List-ID: X-Mailing-List: linux-tegra@vger.kernel.org On Fri, Oct 15, 2021 at 11:14:54AM -0400, Harry Wentland wrote: > > > On 2021-10-15 07:37, Claudio Suarez wrote: > > a) Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. The amdgpu driver still calls > > drm_detect_hdmi_monitor() to retrieve the same information, which > > is less efficient. Change to drm_display_info.is_hdmi > > > > This is a TODO task in Documentation/gpu/todo.rst > > > > b) drm_display_info is updated by drm_get_edid() or > > drm_connector_update_edid_property(). In the amdgpu driver it is almost > > always updated when the edid is read in amdgpu_connector_get_edid(), > > but not always. Change amdgpu_connector_get_edid() and > > amdgpu_connector_free_edid() to keep drm_display_info updated. This allows a) > > to work properly. > > > > c) Use drm_edid_get_monitor_name() instead of duplicating the code that > > parses the EDID in dm_helpers_parse_edid_caps() > > > > Also, remove the unused "struct dc_context *ctx" parameter in > > dm_helpers_parse_edid_caps() > > > > Thanks for this work. > > The fact that you listed three separate changes in this commit > is a clear indication that this patch should be three separate > patches instead. Separating the functional bits from the straight > refactor will help with bisection if this leads to a regression. > > All changes look reasonable to me, though. With this patch split > into three patches in the sequence (b), (c), then (a) this is > Reviewed-by: Harry Wentland Ok, thanks. I'll send three patches. BR Claudio Suarez 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 0CF17C433F5 for ; Sat, 16 Oct 2021 10:16:01 +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 6922E60F9D for ; Sat, 16 Oct 2021 10:16:00 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 6922E60F9D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=net-c.es 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 10C326E429; Sat, 16 Oct 2021 10:16:00 +0000 (UTC) Received: from msg-4.mailo.com (ip-15.mailobj.net [213.182.54.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id B29F06E429; Sat, 16 Oct 2021 10:15:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=net-c.es; s=mailo; t=1634379354; bh=zqWX1gJTKZ5q18uqa5c3Zf7qkP/A+CGNlGSRL6MHo7E=; h=X-EA-Auth:Date:From:To:Cc:Subject:Message-ID:References: MIME-Version:Content-Type:In-Reply-To; b=sMf3huKcgBE4f9Qx46mnqxxW0kG/4lohTFqKS0NtZxNeWRKEMyEaEtphj4CMJ+lgy /yujIldahtXxN2v73vppnv9iR+b7D3zue7w+NHiEgHvg8SjPPoR/slscBcmeAG2zbg QcfsijBw/h/KFQfk1ZZF+RMG9rE7rwfK3oj+PkBY= Received: by b-3.in.mailobj.net [192.168.90.13] with ESMTP via ip-206.mailobj.net [213.182.55.206] Sat, 16 Oct 2021 12:15:54 +0200 (CEST) X-EA-Auth: yKCNRkwyeIQN4zIZhVP/t5GZ4LXBCXpTe707pgk8CcqdT5Z8+L4zajcnVM6jjzg+UKc0YJNBMB/9WGFbSHtL4/w+9RPySOhn Date: Sat, 16 Oct 2021 12:15:51 +0200 From: Claudio Suarez To: Harry Wentland Cc: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-tegra@vger.kernel.org, intel-gfx@lists.freedesktop.org, David Airlie , Daniel Vetter , Laurent Pinchart , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Alex Deucher , Christian =?iso-8859-1?Q?K=F6nig?= , Pan Xinhui , Emma Anholt , Maxime Ripard , Thierry Reding , Patrik Jakobsson , Jingoo Han , Rob Clark , Sean Paul , linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, Chen-Yu Tsai , Sandy Huang , heiko@sntech.de, Neil Armstrong , Robert Foss , Ben Skeggs , nouveau@lists.freedesktop.org Message-ID: References: <20211015113713.630119-1-cssk@net-c.es> <20211015113713.630119-3-cssk@net-c.es> <62a3ee8d-9439-9275-4e71-876b865b9a7d@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <62a3ee8d-9439-9275-4e71-876b865b9a7d@amd.com> Subject: Re: [Nouveau] [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On Fri, Oct 15, 2021 at 11:14:54AM -0400, Harry Wentland wrote: > > > On 2021-10-15 07:37, Claudio Suarez wrote: > > a) Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. The amdgpu driver still calls > > drm_detect_hdmi_monitor() to retrieve the same information, which > > is less efficient. Change to drm_display_info.is_hdmi > > > > This is a TODO task in Documentation/gpu/todo.rst > > > > b) drm_display_info is updated by drm_get_edid() or > > drm_connector_update_edid_property(). In the amdgpu driver it is almost > > always updated when the edid is read in amdgpu_connector_get_edid(), > > but not always. Change amdgpu_connector_get_edid() and > > amdgpu_connector_free_edid() to keep drm_display_info updated. This allows a) > > to work properly. > > > > c) Use drm_edid_get_monitor_name() instead of duplicating the code that > > parses the EDID in dm_helpers_parse_edid_caps() > > > > Also, remove the unused "struct dc_context *ctx" parameter in > > dm_helpers_parse_edid_caps() > > > > Thanks for this work. > > The fact that you listed three separate changes in this commit > is a clear indication that this patch should be three separate > patches instead. Separating the functional bits from the straight > refactor will help with bisection if this leads to a regression. > > All changes look reasonable to me, though. With this patch split > into three patches in the sequence (b), (c), then (a) this is > Reviewed-by: Harry Wentland Ok, thanks. I'll send three patches. BR Claudio Suarez 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 6016AC433FE for ; Sat, 16 Oct 2021 10:16:06 +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 E720760FDA for ; Sat, 16 Oct 2021 10:16:05 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org E720760FDA Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=net-c.es 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 B7B696E431; Sat, 16 Oct 2021 10:16:00 +0000 (UTC) Received: from msg-4.mailo.com (ip-15.mailobj.net [213.182.54.15]) by gabe.freedesktop.org (Postfix) with ESMTPS id B29F06E429; Sat, 16 Oct 2021 10:15:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=net-c.es; s=mailo; t=1634379354; bh=zqWX1gJTKZ5q18uqa5c3Zf7qkP/A+CGNlGSRL6MHo7E=; h=X-EA-Auth:Date:From:To:Cc:Subject:Message-ID:References: MIME-Version:Content-Type:In-Reply-To; b=sMf3huKcgBE4f9Qx46mnqxxW0kG/4lohTFqKS0NtZxNeWRKEMyEaEtphj4CMJ+lgy /yujIldahtXxN2v73vppnv9iR+b7D3zue7w+NHiEgHvg8SjPPoR/slscBcmeAG2zbg QcfsijBw/h/KFQfk1ZZF+RMG9rE7rwfK3oj+PkBY= Received: by b-3.in.mailobj.net [192.168.90.13] with ESMTP via ip-206.mailobj.net [213.182.55.206] Sat, 16 Oct 2021 12:15:54 +0200 (CEST) X-EA-Auth: yKCNRkwyeIQN4zIZhVP/t5GZ4LXBCXpTe707pgk8CcqdT5Z8+L4zajcnVM6jjzg+UKc0YJNBMB/9WGFbSHtL4/w+9RPySOhn Date: Sat, 16 Oct 2021 12:15:51 +0200 From: Claudio Suarez To: Harry Wentland Cc: dri-devel@lists.freedesktop.org, amd-gfx@lists.freedesktop.org, linux-tegra@vger.kernel.org, intel-gfx@lists.freedesktop.org, David Airlie , Daniel Vetter , Laurent Pinchart , Jani Nikula , Joonas Lahtinen , Rodrigo Vivi , Alex Deucher , Christian =?iso-8859-1?Q?K=F6nig?= , Pan Xinhui , Emma Anholt , Maxime Ripard , Thierry Reding , Patrik Jakobsson , Jingoo Han , Rob Clark , Sean Paul , linux-arm-msm@vger.kernel.org, freedreno@lists.freedesktop.org, Chen-Yu Tsai , Sandy Huang , heiko@sntech.de, Neil Armstrong , Robert Foss , Ben Skeggs , nouveau@lists.freedesktop.org Message-ID: References: <20211015113713.630119-1-cssk@net-c.es> <20211015113713.630119-3-cssk@net-c.es> <62a3ee8d-9439-9275-4e71-876b865b9a7d@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <62a3ee8d-9439-9275-4e71-876b865b9a7d@amd.com> Subject: Re: [Intel-gfx] [PATCH 02/15] drm/amdgpu: use drm_* functions instead of duplicated code in amdgpu driver X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Fri, Oct 15, 2021 at 11:14:54AM -0400, Harry Wentland wrote: > > > On 2021-10-15 07:37, Claudio Suarez wrote: > > a) Once EDID is parsed, the monitor HDMI support information is available > > through drm_display_info.is_hdmi. The amdgpu driver still calls > > drm_detect_hdmi_monitor() to retrieve the same information, which > > is less efficient. Change to drm_display_info.is_hdmi > > > > This is a TODO task in Documentation/gpu/todo.rst > > > > b) drm_display_info is updated by drm_get_edid() or > > drm_connector_update_edid_property(). In the amdgpu driver it is almost > > always updated when the edid is read in amdgpu_connector_get_edid(), > > but not always. Change amdgpu_connector_get_edid() and > > amdgpu_connector_free_edid() to keep drm_display_info updated. This allows a) > > to work properly. > > > > c) Use drm_edid_get_monitor_name() instead of duplicating the code that > > parses the EDID in dm_helpers_parse_edid_caps() > > > > Also, remove the unused "struct dc_context *ctx" parameter in > > dm_helpers_parse_edid_caps() > > > > Thanks for this work. > > The fact that you listed three separate changes in this commit > is a clear indication that this patch should be three separate > patches instead. Separating the functional bits from the straight > refactor will help with bisection if this leads to a regression. > > All changes look reasonable to me, though. With this patch split > into three patches in the sequence (b), (c), then (a) this is > Reviewed-by: Harry Wentland Ok, thanks. I'll send three patches. BR Claudio Suarez