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@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/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;