linux-kernel - Re: [PATCH 4/6] drm/rockchip: dw_hdmi: Add vendor hdmi properties

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200813104522.GA6057@pendragon.ideasonboard.com>
Date:   Thu, 13 Aug 2020 13:45:22 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Pekka Paalanen <ppaalanen@...il.com>
Cc:     crj <algea.cao@...k-chips.com>, jernej.skrabec@...l.net,
        laurent.pinchart+renesas@...asonboard.com, jonas@...boo.se,
        airlied@...ux.ie, kuankuan.y@...il.com, narmstrong@...libre.com,
        hjc@...k-chips.com, dri-devel@...ts.freedesktop.org,
        linux-kernel@...r.kernel.org, a.hajda@...sung.com,
        tzimmermann@...e.de, jbrunet@...libre.com,
        linux-rockchip@...ts.infradead.org, darekm@...gle.com,
        sam@...nborg.org, linux-arm-kernel@...ts.infradead.org,
        cychiang@...omium.org
Subject: Re: [PATCH 4/6] drm/rockchip: dw_hdmi: Add vendor hdmi properties

On Thu, Aug 13, 2020 at 10:42:28AM +0300, Pekka Paalanen wrote:
> On Wed, 12 Aug 2020 16:30:17 +0300 Laurent Pinchart wrote:
> > On Wed, Aug 12, 2020 at 07:08:10PM +0800, crj wrote:
> > > 在 2020/8/12 17:33, Laurent Pinchart 写道:  
> > > > On Wed, Aug 12, 2020 at 04:35:43PM +0800, Algea Cao wrote:  
> > > >> 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 = 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@...k-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/gpu/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 @@
> > > >>   
> > > >>   #define HIWORD_UPDATE(val, mask)	(val | (mask) << 16)
> > > >>   
> > > >> +/* 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 ? */
> > > >> +};  
> > > >
> > > > 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).  
> > > 
> > > We use these properties only in Android HW composer, But we can't upstream
> > > 
> > > our HW composer code right now.  Can we use this properties as private 
> > > property
> > > 
> > > and do not upstream HW composer for the time being?  
> > 
> > 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-userspace-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.

Correct. Many userspace projects wouldn't merge a patch before the
kernel API is merged, so that would create a chicken and egg problem :-)

-- 
Regards,

Laurent Pinchart