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;
Date:   Thu, 13 Aug 2020 10:42:28 +0300
From:   Pekka Paalanen <ppaalanen@gmail.com>
To:     crj <algea.cao@rock-chips.com>
Cc:     Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
        jernej.skrabec@siol.net, laurent.pinchart+renesas@ideasonboard.com,
        jonas@kwiboo.se, airlied@linux.ie, kuankuan.y@gmail.com,
        narmstrong@baylibre.com, hjc@rock-chips.com,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        a.hajda@samsung.com, tzimmermann@suse.de, jbrunet@baylibre.com,
        linux-rockchip@lists.infradead.org, darekm@google.com,
        sam@ravnborg.org, linux-arm-kernel@lists.infradead.org,
        cychiang@chromium.org
Subject: Re: [PATCH 4/6] drm/rockchip: dw_hdmi: Add vendor hdmi properties
Message-ID: <20200813104228.096179e7@eldfell>
In-Reply-To: <20200812133017.GJ6057@pendragon.ideasonboard.com>
References: <20200812083120.743-1-algea.cao@rock-chips.com>
        <20200812083543.4231-1-algea.cao@rock-chips.com>
        <20200812093315.GE6057@pendragon.ideasonboard.com>
        <52cca26d-b2b3-22b2-f371-a8086f2e6336@rock-chips.com>
        <20200812133017.GJ6057@pendragon.ideasonboard.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
 boundary="Sig_/=OONQgpVWkCqhFkncTg66Ye"; protocol="application/pgp-signature"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

--Sig_/=OONQgpVWkCqhFkncTg66Ye
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

On Wed, 12 Aug 2020 16:30:17 +0300
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> Hi Algea,
>=20
> On Wed, Aug 12, 2020 at 07:08:10PM +0800, crj wrote:
> > =E5=9C=A8 2020/8/12 17:33, Laurent Pinchart =E5=86=99=E9=81=93: =20
> > > On Wed, Aug 12, 2020 at 04:35:43PM +0800, Algea Cao wrote: =20
> > >> Introduce struct dw_hdmi_property_ops in plat_data to support
> > >> vendor hdmi property.
> > >>
> > >> Implement hdmi vendor properties color_depth_property and
> > >> hdmi_output_property to config hdmi output color depth and
> > >> color format.
> > >>
> > >> The property "hdmi_output_format", the possible value
> > >> could be:
> > >>           - RGB
> > >>           - YCBCR 444
> > >>           - YCBCR 422
> > >>           - YCBCR 420
> > >>
> > >> Default value of the property is set to 0 =3D RGB, so no changes if =
you
> > >> don't set the property.
> > >>
> > >> The property "hdmi_output_depth" possible value could be
> > >>           - Automatic
> > >>             This indicates prefer highest color depth, it is
> > >>             30bit on rockcip platform.
> > >>           - 24bit
> > >>           - 30bit
> > >> The default value of property is 24bit.
> > >>
> > >> Signed-off-by: Algea Cao <algea.cao@rock-chips.com>
> > >> ---
> > >>
> > >>   drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c | 174 ++++++++++++++++=
++++
> > >>   include/drm/bridge/dw_hdmi.h                |  22 +++
> > >>   2 files changed, 196 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c b/drivers/g=
pu/drm/rockchip/dw_hdmi-rockchip.c
> > >> index 23de359a1dec..8f22d9a566db 100644
> > >> --- a/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > >> +++ b/drivers/gpu/drm/rockchip/dw_hdmi-rockchip.c
> > >> @@ -52,6 +52,27 @@
> > >>  =20
> > >>   #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
> > >>  =20
> > >> +/* HDMI output pixel format */
> > >> +enum drm_hdmi_output_type {
> > >> +	DRM_HDMI_OUTPUT_DEFAULT_RGB, /* default RGB */
> > >> +	DRM_HDMI_OUTPUT_YCBCR444, /* YCBCR 444 */
> > >> +	DRM_HDMI_OUTPUT_YCBCR422, /* YCBCR 422 */
> > >> +	DRM_HDMI_OUTPUT_YCBCR420, /* YCBCR 420 */
> > >> +	DRM_HDMI_OUTPUT_YCBCR_HQ, /* Highest subsampled YUV */
> > >> +	DRM_HDMI_OUTPUT_YCBCR_LQ, /* Lowest subsampled YUV */
> > >> +	DRM_HDMI_OUTPUT_INVALID, /* Guess what ? */
> > >> +}; =20
> > >
> > > Vendor-specific properties shouldn't use names starting with drm_ or
> > > DRM_, that's for the DRM core. But this doesn't seem specific to
> > > Rockchip at all, it should be a standard property. Additionally, new
> > > properties need to come with a userspace implementation showing their
> > > usage, in X.org, Weston, the Android DRM/KMS HW composer, or another
> > > relevant upstream project (a test tool is usually not enough). =20
> >=20
> > We use these properties only in Android HW composer, But we can't upstr=
eam
> >=20
> > our HW composer code right now.=C2=A0 Can we use this properties as pri=
vate=20
> > property
> >=20
> > and do not upstream HW composer for the time being? =20
>=20
> It's not my decision, it's a policy of the DRM subsystem to require an
> open implementation in userspace to validate all API additions.

Also read
https://www.kernel.org/doc/html/latest/gpu/drm-uapi.html#open-source-usersp=
ace-requirements
very carefully: it calls for a FOSS userspace project's proper upstream
to have reviewed and accepted the patches to use the new UAPI, but
those patches must NOT be MERGED at that time yet.


Thanks,
pq

--Sig_/=OONQgpVWkCqhFkncTg66Ye
Content-Type: application/pgp-signature
Content-Description: OpenPGP digital signature

-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEEJQjwWQChkWOYOIONI1/ltBGqqqcFAl807uQACgkQI1/ltBGq
qqd/Qg/5AYqVDsoWf/fmcGp7z7OgfKXZM7+iILaEYS5vRXwgTYMG5BWpKlR+fOAn
Umgvjqhnc6GGFaB5S3vZALd7cCMYBoLdaNJ4oR566IjDpXbFl8bagZW3fD8Aaqix
9PzC80muZLPEEKy9zTqLSq2zfiZX41J7lMGTNMSQBpnGfB3YWxKqnKKlVMYtG8sL
mpMq4KfdNLh443IMiu+s4ZkVAPQxj++Bu1BeJnz5Puy1iKY4Wup76tipgNXNHNiG
eHj2hkHrzcXMjPF4JCrPra9OBHhiiv6IxP3fyF0dvfaou+mGzIOeJF5fV3Msna9J
4mYVcEE516sDuUZ8xmUOzwMMPPxqhOqZ/0DQSpU0NP3uevMVJQmcmitNFZMzDFtO
6jXTz7k554GJxrJo9XfSx/QKZPNtCIbQsxW6TKU2ShOCJFs55fO+seOCojaHnr5R
Z0zUzapzyYbmiG1UHjjnyzOVIrQb2Bj2+6NxkQdAdXltKQym1GhdWHE3D/LzhS4R
PZnFJxSU+MMrienr6daNiMfdfSl2vHgfd83qOCeQRXfYlJdzkKktsZfD2YUH0SY3
rq1pHw+5srdW0rKjct15/euFyiH3SUHF88rHYa4MzEC/3z+VASRUwWZ/ThWrRhjb
fajX3c1qqlldIcxv+TknqkFrVuOUV8O9mxP18N4EL4uYCmTlDIc=
=7IHY
-----END PGP SIGNATURE-----

--Sig_/=OONQgpVWkCqhFkncTg66Ye--