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: <b5b44445-8ec4-7d67-149f-ec51ecf1b72c@amd.com>
Date:   Sun, 13 Nov 2022 12:48:28 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.2
Subject: Re: [PATCH] drm/amd/display: don't enable DRM CRTC degamma property
 for DCE
Content-Language: en-US
To:     Melissa Wen <mwen@igalia.com>, harry.wentland@amd.com,
        sunpeng.li@amd.com, alexander.deucher@amd.com,
        christian.koenig@amd.com, Xinhui.Pan@amd.com, airlied@linux.ie,
        daniel@ffwll.ch
Cc:     Nicholas.Choi@amd.com, Nicholas.Kazlauskas@amd.com,
        kernel-dev@igalia.com, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20221103184500.14450-1-mwen@igalia.com>
From:   Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
In-Reply-To: <20221103184500.14450-1-mwen@igalia.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?NGVqU3pwUFRtUEJkY1J2MU1HSHVPeDVwL2MxeVBMcStzZjVaRHphSW1XbEl6?=
 =?utf-8?B?Z3NLblJKN1JSODdJNmw4V1MxWmUrTTFZeEVrTzJ1eEtzYnZiSGk4NXRoVElE?=
 =?utf-8?B?RTdrMjdHUUtWMCtSNUFjaWY2QUNCUXo4UTdhZUlxeVlLMWM2OGQ2Y3RwbHY3?=
 =?utf-8?B?czlQNmJXUVNkS2JvMVVIbmJMTUlGU2tsWW00czJ6enQ1ZWhYUkdyR01KYUZ4?=
 =?utf-8?B?V3VSem84Y2JvSmdzS3p1eGdRMytOS21JaEVpd0EzN2FaaGFBMStmTDJMRzlF?=
 =?utf-8?B?TmVMZXREdXNNNHZ4RjdiNS82QUtPYlVMVGl1ZEYxcFFRSG1MaUZCeHdqU0N1?=
 =?utf-8?B?MUpQOEcwU2JKczBJSVdTTCtqSG8vNHV5SkYyNVpiUmFpMUV0MUlGajlvemg0?=
 =?utf-8?B?TU1vTmtRM0dCd0owbEdqblk1Y2xWRE5IV0pNUWkrTDhLUXhLNHpaNjdjVjRH?=
 =?utf-8?B?SlZTRmd2ZmFuYmZYVUVvZlBqSEJjVGFoeXdHcDJOR3NPRnZuUGZlNVpsRHd6?=
 =?utf-8?B?R3dUc3lzazVQcDZ1UmtkU1RXYzUzbHlTL1puZVpIdTZZNFR1YkVHNEdJNCt1?=
 =?utf-8?B?cHVySWxEcmlJZUo2QXdHVUJBeXd2Qldwd1dmMG1PNENhYzFteERMMVVFeGhS?=
 =?utf-8?B?dDJUWWk3Mi9TYVRiQjN1SGQvbXVHQ0xsWkZjTHU2MHY5U296VnZ0NWJDWG9S?=
 =?utf-8?B?bDhONVZKV1UrMCtVSndSTHIrenBVbXE5cWVXRDJVQTlkUEhzUENMSHRnaVIx?=
 =?utf-8?B?YkxkT3RBYzZ3RjZwSCs0YUJDVGc4SUtMU1BTZ0V6L0JaK3F3MDc3QUw2bG5B?=
 =?utf-8?B?QWJiaXZVZW95V24vZ085c3pINFBBemNHd3pjU0Flcm1lZVZmcXBMQzJDT0VQ?=
 =?utf-8?B?SnkwL2g1WjF4bGEzWnFMOFRBSVJkVy9GV05EcmdvU1RoZk9YdDVJNnUvS1Ux?=
 =?utf-8?B?T2dhTkNFbW9LZ2VlZzE5YUF1NExaRnEySE1RMWc3NWtaT2d4ZEt2MDhYKzl2?=
 =?utf-8?B?cjJVQ2k3SE8zQm5FM00vNUlhNDB3VTlyekNINjlPbldPMVlGSkRRLzJ2QXZV?=
 =?utf-8?B?cElZN2FhaVlWSXIzTzFyYUN4ZXFNcis3dEVPNVZDeTlpOVZoSzJ5MUlPZ0dP?=
 =?utf-8?B?dm1PUEY2S0pNdkFnYzJHc3ZXT1JkWWpaSlhKV05BVUs2elYzUzFPa3NqSVo0?=
 =?utf-8?B?VHduN05hMndMdkw5Q2dHU2I3NG40WTFnN0w4bzhJUGJlcjBXUGx4UnhyUGVu?=
 =?utf-8?B?dWg3UGVFUXcvdzlYdEJaV1FydjN4ZkhYMDBPMTQ0aXlEdWlTang4Q05Sdld2?=
 =?utf-8?B?REVxTVA1Q0RZc3Njb3N6d3VMNU5BSllCdThaZGJ2TDlrRXAyWnB6eTE2ZUJH?=
 =?utf-8?B?ekJrZVVSSGNrN0RFUlVHZCs0RWlLREQvT3U0eDlIVWMxaVMyanZQZVlIUktR?=
 =?utf-8?B?czE5ek9HSXYzMnY1M2pDUlVyQ2oyZHJLSjZlTUN1RlZvMUI2ZkFpTm1VSWQr?=
 =?utf-8?B?disrdkJ5cVFJaWthT2grVjFMcHFBemwrNms5QkZYcUVNVWI1RFg4VGo0ME5m?=
 =?utf-8?B?VDJjQjhrVE5nRjBhZ21wTExWU0VTL3VCMjZXREZxVG9CcHJabE5sWThvQnll?=
 =?utf-8?B?WTcwVTNGZU9jNGhMUGRYM0xaN1I4SkJoVWszUDJ4b3ArQVM2Vm4rdUp4c0Fo?=
 =?utf-8?B?OWJlVVRQbC9vSGVEdlZWcXRtY04xR0RiY0YvM2JTTkI4WDVDWk0rRGQzbXRK?=
 =?utf-8?B?UGFzSkZiakZMMkhER3RveU44anR3dnFLb2lORnhXVjRFMFlocTA3RU5taGl1?=
 =?utf-8?B?WWN6NzhBV2piNnB4YjNVUUVncVBTWWpLeFZlby9lcncwZCt5cTFrZm1MbUh2?=
 =?utf-8?B?c1Z2L1F2RGtsNE01Y1BtcE9yMk1CQ1I0TWk3SnpTTVJKb0psUmRyeExhVEVz?=
 =?utf-8?B?OXc0SEVYS3UrSEh1Y28vL2s0eU9xcFlxT2pKZ1hWS01iN1lnME8vZCtVUFk0?=
 =?utf-8?B?aytKcUtHVFg0QUJZSXBhNTFTeFRQTk5XaEIwb0hTNURoUTBEY1JSaUg4YUJs?=
 =?utf-8?B?NUlQL1Vqdjg0Vms1SmYvb0F6ZSthWTgwQjZHT3hISDVkVHBtc0QydEdQS0ZB?=
 =?utf-8?B?WDRqbkdnWFJJcXMrbWptY0xuT0pCUStvcUFiVnF0dFROTVQvSXkyQjRzK3Jz?=
 =?utf-8?Q?emKrVyqz8WCBBIXcbLsJ7Y4=3D?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 200462d9-9f41-4b71-9ac5-08dac59f28c8
X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB2941.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Nov 2022 17:47:40.3241
 (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: nC/r2oN47rqIte3R543ntKPDbfMayHJA0hAT2PbefqkknHVag706a2iFKUmBvJO73HXWrSFoQ7SSKSO3q0UgGg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4171
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        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


On 11/3/22 14:45, Melissa Wen wrote:
> DM maps DRM CRTC degamma to DPP (pre-blending) degamma block, but DCE doesn't
> support programmable degamma curve anywhere. Currently, a custom degamma is
> accepted by DM but just ignored by DCE driver and degamma correction isn't
> actually applied. There is no way to map custom degamma in DCE, therefore, DRM
> CRTC degamma property shouldn't be enabled for DCE drivers.
> 
> Signed-off-by: Melissa Wen <mwen@igalia.com>
> ---
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 9ac2805c5d63..b3eadfc61555 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -415,7 +415,7 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
>   {
>   	struct amdgpu_crtc *acrtc = NULL;
>   	struct drm_plane *cursor_plane;
> -
> +	bool is_dcn;
>   	int res = -ENOMEM;
>   
>   	cursor_plane = kzalloc(sizeof(*cursor_plane), GFP_KERNEL);
> @@ -453,8 +453,14 @@ int amdgpu_dm_crtc_init(struct amdgpu_display_manager *dm,
>   	acrtc->otg_inst = -1;
>   
>   	dm->adev->mode_info.crtcs[crtc_index] = acrtc;
> -	drm_crtc_enable_color_mgmt(&acrtc->base, MAX_COLOR_LUT_ENTRIES,
> +
> +	/* Don't enable DRM CRTC degamma property for DCE since it doesn't
> +	 * support programmable degamma anywhere.
> +	 */
> +	is_dcn = dm->adev->dm.dc->caps.color.dpp.dcn_arch;
> +	drm_crtc_enable_color_mgmt(&acrtc->base, is_dcn ? MAX_COLOR_LUT_ENTRIES : 0,
>   				   true, MAX_COLOR_LUT_ENTRIES);
> +
>   	drm_mode_crtc_set_gamma_size(&acrtc->base, MAX_COLOR_LEGACY_LUT_ENTRIES);
>   
>   	return 0;

Hi,

I tested it in a DCE device and the patch lgtm.

Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>

and merged it into amd-staging-drm-next.

Thanks
Siqueira