Received: by 2002:a05:6a10:206:0:0:0:0 with SMTP id 6csp5442915pxj; Wed, 23 Jun 2021 01:05:38 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzI+G1CjNqbO+g+0pyQbRIhHgo+EkqGIHrj3RkoRxg8Y6Mb2oVcSDFdkxU5JMxOQ4KnTzWt X-Received: by 2002:a05:6402:1450:: with SMTP id d16mr10581557edx.274.1624435537895; Wed, 23 Jun 2021 01:05:37 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1624435537; cv=none; d=google.com; s=arc-20160816; b=Qs9TK9zObbHdwhKXX2kVWancqoB4hmLfVNHBjU3DJ4S45Bdd40zuJr8Bkyfwlw/lQS Wb1rxT+jrps5DmV90MB+urRgxDZkd2JXDLT/xazs3mBhvFYamtcCL8YoOpo4j3MgLh9A Oh86ddqN4Nx0PpZRcFayea7UOiOsCi4pmPEjph6Wh4vSo1dH8WWcwUJw0z3OjnNIIN2c XtRecLa6XGFPxfy65J/tdZcx41rEj4yuRvbw/2dlAC7ejhG1vGlI95HCj8KKZdtGYkEz 9rjhKpS2A1f5o3yPm2B47+JTj4Lx/lzt0s1xsH5DqOEVJz57w0ZV0rPKtlXBJL8ti5fC n7YA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:mime-version:references:in-reply-to:message-id :subject:cc:to:from:date:dkim-signature; bh=ALp5/N27RPVlOOdmQTHFA/PlRSYSgcTj/HOCoALwZHU=; b=PE45/fnejKJxwW+l3MyoI4uYt0pWmi9EFKDhUK7IObwfD5UTaWlwA9qswZRcXrOjKD /bvDxqCuXmw+1tI0zIv1u47BZr047aL6XW1jUJP/JZMIMA75T0G//ShzskJeGyDdtoPz 0vBn7JF0m4la1GYLPy2p8VJfBMnkAvxWzO2A45RKelrSUK1B9H3DUTnqeukNahENB1oq CDhoAznSPCUVe4LlCk4whxMxjLnTUXbCzUjQAlJVtroWfY2DWo/PeuZW1ATUSGI/X6Gt /aVm0Gj9byjLvvIMl+885VUEGM4C+htl4O50Iv3nuuF+h04q5oNzGCRpOJ5fYAMinKLY rJjw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=jTbDpOhk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [23.128.96.18]) by mx.google.com with ESMTP id zp1si6776590ejb.452.2021.06.23.01.05.15; Wed, 23 Jun 2021 01:05:37 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20161025 header.b=jTbDpOhk; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230121AbhFWIEU (ORCPT + 99 others); Wed, 23 Jun 2021 04:04:20 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:56218 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229915AbhFWIET (ORCPT ); Wed, 23 Jun 2021 04:04:19 -0400 Received: from mail-lf1-x133.google.com (mail-lf1-x133.google.com [IPv6:2a00:1450:4864:20::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 6CDA7C061574 for ; Wed, 23 Jun 2021 01:02:02 -0700 (PDT) Received: by mail-lf1-x133.google.com with SMTP id i1so2612054lfe.6 for ; Wed, 23 Jun 2021 01:02:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version; bh=ALp5/N27RPVlOOdmQTHFA/PlRSYSgcTj/HOCoALwZHU=; b=jTbDpOhkkcRBj+Tr+txKS3w8AL191MGPqQKTJnQWRFyghzbRxc50ybjVs9+xSyTvBP NPKTd7wql5N78Irn8b3ZVHQHFY0FjPEmEsiOLtkdnErfTMzbrdg/Oxhn6tfTI03wP8pD 4OxyMTgkHuIi/5O5pBE/DFuXlbiJyQoo8NpmtaGXEZgp8yvXA14xeAiJS2uNsTsyMJNs U2fwbQiqPFzTc1K164Xm2DcFlE3uTI1PF6xJc8NR7rrQabMu0T9kSMPPTLF+S1JmnW8O xZ143ju579UWLalmW98hQWyCtdk8LrFHsLTMBaxd6b3Bd+8teorXbhCb1Eg0/zqsok6Y IQ8w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version; bh=ALp5/N27RPVlOOdmQTHFA/PlRSYSgcTj/HOCoALwZHU=; b=G+Eiv1o+v3ID+HGGNhuSu94a6w2EFQdcGuCl4os2ip5JYsTvDr1b5IHu+D31Sp5znu G3T7kQWYnEXJfwIYC76YEf8xt+7Z0n32z+HLZxWhSJCkEvR3ylJu1ynLikxFoPIOmJkZ uo7jsN2bNj0DUE+QY/CJ2j+Vrn+eK2nm4HSLUzcFpbXpMHKw0cenvPjot/dbp0+IZPFV c/N2qBVdCUhKS92dIqCEFhZvBTijIGFPh4a7mLIvj0aZ/TdsId85oDNr0epYi8aey0/4 XzhdKtwenNGznFWdURU2NHp1KgzOUaTuzoenNVfeI+70Gq3nrRnnp5ec2d8wn0sLbj8C pRkg== X-Gm-Message-State: AOAM531C+R0AHNYCFG1SgCFdjr4H2QmpB7+Q6o4Dl8PcTUo7I6Lx+/SP 5GaqfAcyifXN53ocda5pg4g= X-Received: by 2002:a05:6512:4c3:: with SMTP id w3mr6328062lfq.594.1624435320753; Wed, 23 Jun 2021 01:02:00 -0700 (PDT) Received: from eldfell ([194.136.85.206]) by smtp.gmail.com with ESMTPSA id z15sm2459818lfs.207.2021.06.23.01.01.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 23 Jun 2021 01:02:00 -0700 (PDT) Date: Wed, 23 Jun 2021 11:01:56 +0300 From: Pekka Paalanen To: Werner Sembach Cc: amd-gfx@lists.freedesktop.org, tzimmermann@suse.de, intel-gfx@lists.freedesktop.org, sunpeng.li@amd.com, dri-devel@lists.freedesktop.org, joonas.lahtinen@linux.intel.com, maarten.lankhorst@linux.intel.com, linux-kernel@vger.kernel.org, mripard@kernel.org, airlied@linux.ie, jani.nikula@linux.intel.com, daniel@ffwll.ch, rodrigo.vivi@intel.com, alexander.deucher@amd.com, harry.wentland@amd.com, christian.koenig@amd.com Subject: Re: [PATCH v4 17/17] drm/amd/display: Add handling for new "Broadcast RGB" property Message-ID: <20210623110156.4791505e@eldfell> In-Reply-To: References: <20210618091116.14428-1-wse@tuxedocomputers.com> <20210618091116.14428-18-wse@tuxedocomputers.com> <20210622102955.1e0488b1@eldfell> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: multipart/signed; boundary="Sig_/9t65WvBL3CHONVstlbG_ova"; protocol="application/pgp-signature"; micalg=pgp-sha256 Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --Sig_/9t65WvBL3CHONVstlbG_ova Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable On Tue, 22 Jun 2021 11:28:57 +0200 Werner Sembach wrote: > Am 22.06.21 um 09:29 schrieb Pekka Paalanen: > > On Fri, 18 Jun 2021 11:11:16 +0200 > > Werner Sembach wrote: > > =20 > >> This commit implements the "Broadcast RGB" drm property for the AMD GPU > >> driver. > >> > >> Signed-off-by: Werner Sembach > >> --- > >> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 22 ++++++++++++++----- > >> .../display/amdgpu_dm/amdgpu_dm_mst_types.c | 4 ++++ > >> 2 files changed, 21 insertions(+), 5 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drive= rs/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > >> index 9ffd2f9d3d75..c5dbf948a47a 100644 > >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > >> @@ -5252,7 +5252,8 @@ get_aspect_ratio(const struct drm_display_mode *= mode_in) > >> } > >> =20 > >> static enum dc_color_space > >> -get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing) > >> +get_output_color_space(const struct dc_crtc_timing *dc_crtc_timing, > >> + enum drm_mode_color_range preferred_color_range) > >> { > >> enum dc_color_space color_space =3D COLOR_SPACE_SRGB; > >> =20 > >> @@ -5267,13 +5268,17 @@ get_output_color_space(const struct dc_crtc_ti= ming *dc_crtc_timing) > >> * respectively > >> */ > >> if (dc_crtc_timing->pix_clk_100hz > 270300) { > >> - if (dc_crtc_timing->flags.Y_ONLY) > >> + if (dc_crtc_timing->flags.Y_ONLY > >> + || preferred_color_range =3D=3D > >> + DRM_MODE_COLOR_RANGE_LIMITED_16_235) > >> color_space =3D > >> COLOR_SPACE_YCBCR709_LIMITED; > >> else > >> color_space =3D COLOR_SPACE_YCBCR709; =20 > > Hi, > > > > does this mean that amdgpu would be using a property named "Broadcast > > RGB" to control the range of YCbCr too? =20 >=20 > Yes, because I avoided creating a new property, but I'm not really happy = with it either. >=20 > Possibility 1: Use "Broadcast RGB" for Y'CbCr too and clarify in document= ation > =C2=A0=C2=A0=C2=A0 - still confusing name > =C2=A0=C2=A0=C2=A0 - limited does not mean something a little bit differe= nt for Y'CbCr and not strictly 16-235: > https://www.kernel.org/doc/html/v5.12/userspace-api/media/v4l/colorspaces= -defs.html#c.V4L.v4l2_quantization , but name > of option is given by preexisting property >=20 > Possibility 2: Deprecate "Broadcast RGB" and a a more neutral sounding "p= referred color range", with the more neutral > sounding "limited" option instead of "Limited 16:235" > =C2=A0=C2=A0=C2=A0 - What's the relation between the 2? pq mentioned on t= he amdgpu > gitlab that there is a posibility for userspace to have only the new > or the old one shown It's just an idea that we could decide to expose only one or the other property. It would need to be engineered in code, go through the UAPI validation with userspace etc. I'm not aware of this being done before exactly like this, but DRM client caps exist. > =C2=A0=C2=A0=C2=A0 - Alternatively ignore "Broadcast RGB" when "preferred= color range" is set and have them coexist? Determining "is set" means we would need "unset" value for "preferred color range". But there is no notion of who set it. If some KMS client decides to set it, then it will likely remain set, even if you next start another KMS client who does not use this property - it would just confuse users when "Broadcast RGB" silently stopped working while it still exists. So I don't think this is a good solution. When considering a new property, what I wrote just earlier fit here: https://lists.freedesktop.org/archives/dri-devel/2021-June/312248.html There are more questions that just what does the limited range actually mean. > > That is surprising. If this is truly wanted, then the documentation of > > "Broadcast RGB" must say that it applies to YCbCr too. > > > > Does amdgpu do the same as intel wrt. to the question about whose > > responsibility it is to make the pixels at the connector to match the > > set range? =20 >=20 > I guess the kernel driver does the conversion, but i have to check > for both. >=20 > For Intel I did not change the behavior of Boradcast RGB, but i think > it's not clearly specified in the docs where the conversion happens. Right, at the very least the current behaviour needs to be documented before enrolling this property to any more drivers, so that those drivers can then be reviewed to work the same way. You notice I didn't actually answer your question 1 or 2. I don't know. Going with 1 is easy compared to 2, even if the names are awkward but it technically shouldn't cause any problems. 2 may or may not be better, and until we have answers to which design is better, it's maybe best to leave option 2 alone? Thanks, pq --Sig_/9t65WvBL3CHONVstlbG_ova Content-Type: application/pgp-signature Content-Description: OpenPGP digital signature -----BEGIN PGP SIGNATURE----- iQIzBAEBCAAdFiEEJQjwWQChkWOYOIONI1/ltBGqqqcFAmDS6nQACgkQI1/ltBGq qqchjQ//UhWP/scpD6nbt+KCsdHLrYGMFqmi8iFTG/Zhl+XDBib1UkOYt0zH5IVo i9Ag05LEaNmGK51xuDZ65wAyjyEY66T3tvcZyM6Wb+y+Au7DWTTEA5v3I4VmTD87 tSHHvAFEKtgUdukSpoemhEUuxlCad/sOqLbA0D/ykX2osM7eLiy3ViSZdjxT0Nyt 0KIhgfVQ4t5Df/uzjRHOISj6kzgUx/pZrBwwY/9c7vZ5hIzkJahEmYhOxXggMX4w beZPHa12peZ+zom8Y94aW51WK85qoFYJ8F9Lq+Yb0NcMiKpK5qk+u4n1We1obbuG KePy6Ctwbz2QzDsi2PBIpHHYIdEh3mZpQa8RU4bWIHYFaI6aAg/CMR46jnUMzjrw DklDzxcd/l5/HnokBVRbHjTrks0J379KY6o85ZWyg4cUJOhcbmPEC24H7NDUpZzd pq6LctfNTEMMeOB8DY975tNr3UYaR7hfP00iXGUC+15s0buqBhKFMcfyZ4O6dGyo /xjX35DrGoB31aOQdLevPh6Nd+682PmwwrtAGAC/h9T+NQ2X2B7WrirBDhkhxlC7 PW7sHpyFpLBInTjOuiavpV8bd6M/cd0nONzxJQTrtw5TP7c8kuGGdC4vYplW5gtT aPUsotdeQxloZvgk8oewjvHuIB/acDz+5GDM/4YhIpjCAH2ee80= =9bA4 -----END PGP SIGNATURE----- --Sig_/9t65WvBL3CHONVstlbG_ova--