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@xxxxxxxxxxxxxx> > --- > > 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel