Re: [PATCH v2 1/2] drm: bridge: dw-hdmi: Take input format from plat_data

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Neil,

Thank you for the patch.

On Thursday 02 Mar 2017 16:29:31 Neil Armstrong wrote:
> Some display pipelines can only provide non-RBG input pixels to the HDMI TX
> Controller, this patch takes the pixel format from the plat_data if
> provided.
> 
> Signed-off-by: Neil Armstrong <narmstrong@xxxxxxxxxxxx>
> ---
>  drivers/gpu/drm/bridge/dw-hdmi.c | 59 ++++++++++++++++++-------------------
>  include/drm/bridge/dw_hdmi.h     |  9 ++++++
>  2 files changed, 39 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c
> b/drivers/gpu/drm/bridge/dw-hdmi.c index 026a0dc..653ecd7 100644
> --- a/drivers/gpu/drm/bridge/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/dw-hdmi.c
> @@ -35,12 +35,6 @@
> 
>  #define HDMI_EDID_LEN		512
> 
> -#define RGB			0
> -#define YCBCR444		1
> -#define YCBCR422_16BITS		2
> -#define YCBCR422_8BITS		3
> -#define XVYCC444		4
> -
>  enum hdmi_datamap {
>  	RGB444_8B = 0x01,
>  	RGB444_10B = 0x03,
> @@ -94,8 +88,8 @@ struct hdmi_vmode {
>  };
> 
>  struct hdmi_data_info {
> -	unsigned int enc_in_format;
> -	unsigned int enc_out_format;
> +	enum dw_hdmi_color_enc_format enc_in_format;
> +	enum dw_hdmi_color_enc_format enc_out_format;
>  	unsigned int enc_color_depth;
>  	unsigned int colorimetry;
>  	unsigned int pix_repet_factor;
> @@ -569,7 +563,7 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
>  	int color_format = 0;
>  	u8 val;
> 
> -	if (hdmi->hdmi_data.enc_in_format == RGB) {
> +	if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_RGB) {
>  		if (hdmi->hdmi_data.enc_color_depth == 8)
>  			color_format = 0x01;
>  		else if (hdmi->hdmi_data.enc_color_depth == 10)
> @@ -580,7 +574,7 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
>  			color_format = 0x07;
>  		else
>  			return;
> -	} else if (hdmi->hdmi_data.enc_in_format == YCBCR444) {
> +	} else if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_YCBCR444) 
{
>  		if (hdmi->hdmi_data.enc_color_depth == 8)
>  			color_format = 0x09;
>  		else if (hdmi->hdmi_data.enc_color_depth == 10)
> @@ -591,7 +585,8 @@ static void hdmi_video_sample(struct dw_hdmi *hdmi)
>  			color_format = 0x0F;
>  		else
>  			return;
> -	} else if (hdmi->hdmi_data.enc_in_format == YCBCR422_8BITS) {
> +	} else if (hdmi->hdmi_data.enc_in_format ==
> +					DW_HDMI_ENC_FMT_YCBCR422_8BITS) {
>  		if (hdmi->hdmi_data.enc_color_depth == 8)
>  			color_format = 0x16;
>  		else if (hdmi->hdmi_data.enc_color_depth == 10)
> @@ -627,20 +622,20 @@ static int is_color_space_conversion(struct dw_hdmi
> *hdmi)
> 
>  static int is_color_space_decimation(struct dw_hdmi *hdmi)
>  {
> -	if (hdmi->hdmi_data.enc_out_format != YCBCR422_8BITS)
> +	if (hdmi->hdmi_data.enc_out_format != DW_HDMI_ENC_FMT_YCBCR422_8BITS)
>  		return 0;
> -	if (hdmi->hdmi_data.enc_in_format == RGB ||
> -	    hdmi->hdmi_data.enc_in_format == YCBCR444)
> +	if (hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_RGB ||
> +	    hdmi->hdmi_data.enc_in_format == DW_HDMI_ENC_FMT_YCBCR444)
>  		return 1;
>  	return 0;
>  }
> 
>  static int is_color_space_interpolation(struct dw_hdmi *hdmi)
>  {
> -	if (hdmi->hdmi_data.enc_in_format != YCBCR422_8BITS)
> +	if (hdmi->hdmi_data.enc_in_format != DW_HDMI_ENC_FMT_YCBCR422_8BITS)
>  		return 0;
> -	if (hdmi->hdmi_data.enc_out_format == RGB ||
> -	    hdmi->hdmi_data.enc_out_format == YCBCR444)
> +	if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_RGB ||
> +	    hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_YCBCR444)
>  		return 1;
>  	return 0;
>  }
> @@ -652,13 +647,14 @@ static void dw_hdmi_update_csc_coeffs(struct dw_hdmi
> *hdmi) u32 csc_scale = 1;
> 
>  	if (is_color_space_conversion(hdmi)) {
> -		if (hdmi->hdmi_data.enc_out_format == RGB) {
> +		if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_RGB) {
>  			if (hdmi->hdmi_data.colorimetry ==
>  					HDMI_COLORIMETRY_ITU_601)
>  				csc_coeff = &csc_coeff_rgb_out_eitu601;
>  			else
>  				csc_coeff = &csc_coeff_rgb_out_eitu709;
> -		} else if (hdmi->hdmi_data.enc_in_format == RGB) {
> +		} else if (hdmi->hdmi_data.enc_in_format ==
> +					DW_HDMI_ENC_FMT_RGB) {
>  			if (hdmi->hdmi_data.colorimetry ==
>  					HDMI_COLORIMETRY_ITU_601)
>  				csc_coeff = &csc_coeff_rgb_in_eitu601;
> @@ -730,8 +726,8 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
>  	struct hdmi_data_info *hdmi_data = &hdmi->hdmi_data;
>  	u8 val, vp_conf;
> 
> -	if (hdmi_data->enc_out_format == RGB ||
> -	    hdmi_data->enc_out_format == YCBCR444) {
> +	if (hdmi_data->enc_out_format == DW_HDMI_ENC_FMT_RGB ||
> +	    hdmi_data->enc_out_format == DW_HDMI_ENC_FMT_YCBCR444) {
>  		if (!hdmi_data->enc_color_depth) {
>  			output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS;
>  		} else if (hdmi_data->enc_color_depth == 8) {
> @@ -746,7 +742,8 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi)
>  		} else {
>  			return;
>  		}
> -	} else if (hdmi_data->enc_out_format == YCBCR422_8BITS) {
> +	} else if (hdmi_data->enc_out_format ==
> +					DW_HDMI_ENC_FMT_YCBCR422_8BITS) {
>  		if (!hdmi_data->enc_color_depth ||
>  		    hdmi_data->enc_color_depth == 8)
>  			remap_size = HDMI_VP_REMAP_YCC422_16bit;
> @@ -1138,15 +1135,16 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi,
> struct drm_display_mode *mode) /* Initialise info frame from DRM mode */
>  	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode);
> 
> -	if (hdmi->hdmi_data.enc_out_format == YCBCR444)
> +	if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_YCBCR444)
>  		frame.colorspace = HDMI_COLORSPACE_YUV444;
> -	else if (hdmi->hdmi_data.enc_out_format == YCBCR422_8BITS)
> +	else if (hdmi->hdmi_data.enc_out_format ==
> +					DW_HDMI_ENC_FMT_YCBCR422_8BITS)
>  		frame.colorspace = HDMI_COLORSPACE_YUV422;
>  	else
>  		frame.colorspace = HDMI_COLORSPACE_RGB;
> 
>  	/* Set up colorimetry */
> -	if (hdmi->hdmi_data.enc_out_format == XVYCC444) {
> +	if (hdmi->hdmi_data.enc_out_format == DW_HDMI_ENC_FMT_XVYCC444) {
>  		frame.colorimetry = HDMI_COLORIMETRY_EXTENDED;
>  		if (hdmi->hdmi_data.colorimetry == HDMI_COLORIMETRY_ITU_601)
>  			frame.extended_colorimetry =
> @@ -1154,7 +1152,7 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi,
> struct drm_display_mode *mode) else /*hdmi->hdmi_data.colorimetry ==
> HDMI_COLORIMETRY_ITU_709*/ frame.extended_colorimetry =
>  				HDMI_EXTENDED_COLORIMETRY_XV_YCC_709;
> -	} else if (hdmi->hdmi_data.enc_out_format != RGB) {
> +	} else if (hdmi->hdmi_data.enc_out_format != DW_HDMI_ENC_FMT_RGB) {
>  		frame.colorimetry = hdmi->hdmi_data.colorimetry;
>  		frame.extended_colorimetry = 
HDMI_EXTENDED_COLORIMETRY_XV_YCC_601;
>  	} else { /* Carries no data */
> @@ -1443,10 +1441,13 @@ static int dw_hdmi_setup(struct dw_hdmi *hdmi,
> struct drm_display_mode *mode)
> hdmi->hdmi_data.video_mode.mpixelrepetitionoutput = 0;
>  	hdmi->hdmi_data.video_mode.mpixelrepetitioninput = 0;
> 
> -	/* TODO: Get input format from IPU (via FB driver interface) */
> -	hdmi->hdmi_data.enc_in_format = RGB;
> +	/* Get input format from plat data or fallback to RGB */
> +	if (hdmi->plat_data->input_fmt >= 0)
> +		hdmi->hdmi_data.enc_in_format = hdmi->plat_data->input_fmt;
> +	else
> +		hdmi->hdmi_data.enc_in_format = DW_HDMI_ENC_FMT_RGB;
> 
> -	hdmi->hdmi_data.enc_out_format = RGB;
> +	hdmi->hdmi_data.enc_out_format = DW_HDMI_ENC_FMT_RGB;
> 
>  	hdmi->hdmi_data.enc_color_depth = 8;
>  	hdmi->hdmi_data.pix_repet_factor = 0;
> diff --git a/include/drm/bridge/dw_hdmi.h b/include/drm/bridge/dw_hdmi.h
> index bcceee8..8c0cc13 100644
> --- a/include/drm/bridge/dw_hdmi.h
> +++ b/include/drm/bridge/dw_hdmi.h
> @@ -21,6 +21,14 @@ enum {
>  	DW_HDMI_RES_MAX,
>  };
> 
> +enum dw_hdmi_color_enc_format {
> +	DW_HDMI_ENC_FMT_RGB = 0,
> +	DW_HDMI_ENC_FMT_YCBCR444,
> +	DW_HDMI_ENC_FMT_YCBCR422_16BITS,
> +	DW_HDMI_ENC_FMT_YCBCR422_8BITS,
> +	DW_HDMI_ENC_FMT_XVYCC444,
> +};
> +
>  enum dw_hdmi_phy_type {
>  	DW_HDMI_PHY_DWC_HDMI_TX_PHY = 0x00,
>  	DW_HDMI_PHY_DWC_MHL_PHY_HEAC = 0xb2,
> @@ -62,6 +70,7 @@ struct dw_hdmi_plat_data {
>  	struct regmap *regm;
>  	enum drm_mode_status (*mode_valid)(struct drm_connector *connector,
>  					   struct drm_display_mode *mode);
> +	enum dw_hdmi_color_enc_format input_fmt;

As reported before, I think we should use a combination of media bus format 
and color space information instead of a custom enum. Ideally this should also 
be queried at runtime, but that could be done in a second step.

> 
>  	/* Vendor PHY support */
>  	const struct dw_hdmi_phy_ops *phy_ops;

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux