| 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
| ||
|
Date: Wed, 12 Aug 2020 12:33:15 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Algea Cao <algea.cao@...k-chips.com>
Cc: a.hajda@...sung.com, kuankuan.y@...il.com, hjc@...k-chips.com,
tzimmermann@...e.de, dri-devel@...ts.freedesktop.org,
sam@...nborg.org, airlied@...ux.ie, heiko@...ech.de,
jernej.skrabec@...l.net, laurent.pinchart+renesas@...asonboard.com,
jonas@...boo.se, mripard@...nel.org, darekm@...gle.com,
linux-rockchip@...ts.infradead.org,
linux-arm-kernel@...ts.infradead.org, cychiang@...omium.org,
linux-kernel@...r.kernel.org, narmstrong@...libre.com,
jbrunet@...libre.com, maarten.lankhorst@...ux.intel.com,
daniel@...ll.ch
Subject: Re: [PATCH 4/6] drm/rockchip: dw_hdmi: Add vendor hdmi properties
Hi Algea,
Thank you for the patch.
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).
> +
> +enum dw_hdmi_rockchip_color_depth {
> + ROCKCHIP_HDMI_DEPTH_8,
> + ROCKCHIP_HDMI_DEPTH_10,
> + ROCKCHIP_HDMI_DEPTH_12,
> + ROCKCHIP_HDMI_DEPTH_16,
> + ROCKCHIP_HDMI_DEPTH_420_10,
> + ROCKCHIP_HDMI_DEPTH_420_12,
> + ROCKCHIP_HDMI_DEPTH_420_16
> +};
> +
> /**
> * struct rockchip_hdmi_chip_data - splite the grf setting of kind of chips
> * @lcdsel_grf_reg: grf register offset of lcdc select
> @@ -73,6 +94,12 @@ struct rockchip_hdmi {
> struct clk *grf_clk;
> struct dw_hdmi *hdmi;
> struct phy *phy;
> +
> + struct drm_property *color_depth_property;
> + struct drm_property *hdmi_output_property;
> +
> + unsigned int colordepth;
> + enum drm_hdmi_output_type hdmi_output;
> };
>
> #define to_rockchip_hdmi(x) container_of(x, struct rockchip_hdmi, x)
> @@ -327,6 +354,150 @@ static void dw_hdmi_rockchip_genphy_disable(struct dw_hdmi *dw_hdmi, void *data)
> phy_power_off(hdmi->phy);
> }
>
> +static const struct drm_prop_enum_list color_depth_enum_list[] = {
> + { 0, "Automatic" }, /* Prefer highest color depth */
> + { 8, "24bit" },
> + { 10, "30bit" },
> +};
> +
> +static const struct drm_prop_enum_list drm_hdmi_output_enum_list[] = {
> + { DRM_HDMI_OUTPUT_DEFAULT_RGB, "output_rgb" },
> + { DRM_HDMI_OUTPUT_YCBCR444, "output_ycbcr444" },
> + { DRM_HDMI_OUTPUT_YCBCR422, "output_ycbcr422" },
> + { DRM_HDMI_OUTPUT_YCBCR420, "output_ycbcr420" },
> + { DRM_HDMI_OUTPUT_YCBCR_HQ, "output_ycbcr_high_subsampling" },
> + { DRM_HDMI_OUTPUT_YCBCR_LQ, "output_ycbcr_low_subsampling" },
> + { DRM_HDMI_OUTPUT_INVALID, "invalid_output" },
> +};
> +
> +static void
> +dw_hdmi_rockchip_attach_properties(struct drm_connector *connector,
> + unsigned int color, int version,
> + void *data)
> +{
> + struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
> + struct drm_property *prop;
> +
> + switch (color) {
> + case MEDIA_BUS_FMT_RGB101010_1X30:
> + hdmi->hdmi_output = DRM_HDMI_OUTPUT_DEFAULT_RGB;
> + hdmi->colordepth = 10;
> + break;
> + case MEDIA_BUS_FMT_YUV8_1X24:
> + hdmi->hdmi_output = DRM_HDMI_OUTPUT_YCBCR444;
> + hdmi->colordepth = 8;
> + break;
> + case MEDIA_BUS_FMT_YUV10_1X30:
> + hdmi->hdmi_output = DRM_HDMI_OUTPUT_YCBCR444;
> + hdmi->colordepth = 10;
> + break;
> + case MEDIA_BUS_FMT_UYVY10_1X20:
> + hdmi->hdmi_output = DRM_HDMI_OUTPUT_YCBCR422;
> + hdmi->colordepth = 10;
> + break;
> + case MEDIA_BUS_FMT_UYVY8_1X16:
> + hdmi->hdmi_output = DRM_HDMI_OUTPUT_YCBCR422;
> + hdmi->colordepth = 8;
> + break;
> + case MEDIA_BUS_FMT_UYYVYY8_0_5X24:
> + hdmi->hdmi_output = DRM_HDMI_OUTPUT_YCBCR420;
> + hdmi->colordepth = 8;
> + break;
> + case MEDIA_BUS_FMT_UYYVYY10_0_5X30:
> + hdmi->hdmi_output = DRM_HDMI_OUTPUT_YCBCR420;
> + hdmi->colordepth = 10;
> + break;
> + default:
> + hdmi->hdmi_output = DRM_HDMI_OUTPUT_DEFAULT_RGB;
> + hdmi->colordepth = 8;
> + }
> +
> + prop = drm_property_create_enum(connector->dev, 0,
> + "hdmi_output_depth",
> + color_depth_enum_list,
> + ARRAY_SIZE(color_depth_enum_list));
> + if (prop) {
> + hdmi->color_depth_property = prop;
> + drm_object_attach_property(&connector->base, prop, 0);
> + }
> +
> + prop = drm_property_create_enum(connector->dev, 0, "hdmi_output_format",
> + drm_hdmi_output_enum_list,
> + ARRAY_SIZE(drm_hdmi_output_enum_list));
> + if (prop) {
> + hdmi->hdmi_output_property = prop;
> + drm_object_attach_property(&connector->base, prop, 0);
> + }
> +}
> +
> +static void
> +dw_hdmi_rockchip_destroy_properties(struct drm_connector *connector,
> + void *data)
> +{
> + struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
> +
> + if (hdmi->color_depth_property) {
> + drm_property_destroy(connector->dev,
> + hdmi->color_depth_property);
> + hdmi->color_depth_property = NULL;
> + }
> +
> + if (hdmi->hdmi_output_property) {
> + drm_property_destroy(connector->dev,
> + hdmi->hdmi_output_property);
> + hdmi->hdmi_output_property = NULL;
> + }
> +}
> +
> +static int
> +dw_hdmi_rockchip_set_property(struct drm_connector *connector,
> + struct drm_connector_state *state,
> + struct drm_property *property,
> + u64 val,
> + void *data)
> +{
> + struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
> +
> + if (property == hdmi->color_depth_property) {
> + hdmi->colordepth = val;
> + return 0;
> + } else if (property == hdmi->hdmi_output_property) {
> + hdmi->hdmi_output = val;
> + return 0;
> + }
> +
> + DRM_ERROR("failed to set rockchip hdmi connector property\n");
> + return -EINVAL;
> +}
> +
> +static int
> +dw_hdmi_rockchip_get_property(struct drm_connector *connector,
> + const struct drm_connector_state *state,
> + struct drm_property *property,
> + u64 *val,
> + void *data)
> +{
> + struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
> +
> + if (property == hdmi->color_depth_property) {
> + *val = hdmi->colordepth;
> + return 0;
> + } else if (property == hdmi->hdmi_output_property) {
> + *val = hdmi->hdmi_output;
> + return 0;
> + }
> +
> + DRM_ERROR("failed to get rockchip hdmi connector property\n");
> + return -EINVAL;
> +}
> +
> +static const struct dw_hdmi_property_ops dw_hdmi_rockchip_property_ops = {
> + .attach_properties = dw_hdmi_rockchip_attach_properties,
> + .destroy_properties = dw_hdmi_rockchip_destroy_properties,
> + .set_property = dw_hdmi_rockchip_set_property,
> + .get_property = dw_hdmi_rockchip_get_property,
> +};
> +
> static void dw_hdmi_rk3228_setup_hpd(struct dw_hdmi *dw_hdmi, void *data)
> {
> struct rockchip_hdmi *hdmi = (struct rockchip_hdmi *)data;
> @@ -511,6 +682,9 @@ static int dw_hdmi_rockchip_bind(struct device *dev, struct device *master,
> hdmi->dev = &pdev->dev;
> hdmi->chip_data = plat_data->phy_data;
> plat_data->phy_data = hdmi;
> +
> + plat_data->property_ops = &dw_hdmi_rockchip_property_ops;
> +
> encoder = &hdmi->encoder;
>
> encoder->possible_crtcs = drm_of_find_possible_crtcs(drm, dev->of_node);
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index ea34ca146b82..dc561ebe7a9b 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -6,6 +6,7 @@
> #ifndef __DW_HDMI__
> #define __DW_HDMI__
>
> +#include <drm/drm_property.h>
> #include <sound/hdmi-codec.h>
>
> struct drm_display_info;
> @@ -123,6 +124,24 @@ struct dw_hdmi_phy_ops {
> void (*setup_hpd)(struct dw_hdmi *hdmi, void *data);
> };
>
> +struct dw_hdmi_property_ops {
> + void (*attach_properties)(struct drm_connector *connector,
> + unsigned int color, int version,
> + void *data);
> + void (*destroy_properties)(struct drm_connector *connector,
> + void *data);
> + int (*set_property)(struct drm_connector *connector,
> + struct drm_connector_state *state,
> + struct drm_property *property,
> + u64 val,
> + void *data);
> + int (*get_property)(struct drm_connector *connector,
> + const struct drm_connector_state *state,
> + struct drm_property *property,
> + u64 *val,
> + void *data);
> +};
> +
> struct dw_hdmi_plat_data {
> struct regmap *regm;
>
> @@ -141,6 +160,9 @@ struct dw_hdmi_plat_data {
> const struct drm_display_info *info,
> const struct drm_display_mode *mode);
>
> + /* Vendor Property support */
> + const struct dw_hdmi_property_ops *property_ops;
> +
> /* Vendor PHY support */
> const struct dw_hdmi_phy_ops *phy_ops;
> const char *phy_name;
--
Regards,
Laurent Pinchart
Powered by blists - more mailing lists