| Message ID | 551A629C.5010306@rock-chips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Mar 31, 2015 at 05:02:20AM -0400, Yang Kuankuan wrote: > Besides, as you are going an dw_hdmi cleanups, I want to point another bugs > that relate to the HDMI CTS test. There are somethings wrong with General > Control Pack, as for now the encoder color depth is 8-bit packing mode, > so the color depth only support 24 bits per pixel video, In this case the > CD filed in GCP should set to "Color Depth Not indicated". I'm not sure I follow. According to the iMX6 documentation, setting the CD field to either 0000 or 0100 indicates that the color depth is 24 bits per pixel, 8 bits per component, 8 bit packing mode - there's no documented difference between these. Are you saying that you wish to pass something other than 24 bpp video to your HDMI encoder? > In the end we should keep the *csc_color_depth(HDMI_CSC_SCALE)* & > *color_depth(HDMI_VP_PR_CD)* to zero, code should modify like this GCP > would test pass: From what you're describing, you want CD field = 0 and CSC_SCALE = 0. It looks like hdmi_video_packetize() will set the CD field to zero if hdmi_data.enc_color_depth = 0, but that would cause hdmi_video_sample() and hdmi_video_csc() to fail. Maybe those two functions should be fixed to accept a color depth of zero, and maybe you need to set enc_color_depth to 0? Interestingly, HDMI_CSC_SCALE_CSC_COLORDE_PTH_24BPP is defined to be zero, but again, in the iMX6 data, it could take a value of either 0x00 or 0x40. I think hdmi_video_csc() should set this to 0x40 if hdmi_data.enc_color_depth = 0, and 0x40 for hdmi_data.enc_color_depth = 8.
Hi Russell, ? 2015/3/31 19:57, Russell King - ARM Linux ??: > On Tue, Mar 31, 2015 at 05:02:20AM -0400, Yang Kuankuan wrote: >> Besides, as you are going an dw_hdmi cleanups, I want to point another bugs >> that relate to the HDMI CTS test. There are somethings wrong with General >> Control Pack, as for now the encoder color depth is 8-bit packing mode, >> so the color depth only support 24 bits per pixel video, In this case the >> CD filed in GCP should set to "Color Depth Not indicated". > I'm not sure I follow. > > According to the iMX6 documentation, setting the CD field to either 0000 > or 0100 indicates that the color depth is 24 bits per pixel, 8 bits per > component, 8 bit packing mode - there's no documented difference between > these. Yeah, that is the point. Though there's no designedware datasheet difference between "0000b" & 0100b" in vp_pr_cd, but I think the color_depth in vp_pr_cd register are mapping to CD0-CD3 filed in GCP packet, and acutally "0000b" & "0100b" are differnent in CD filed ("0000b Color Depth not indicated" & "0100b 24 bit per pixel"). If the CD filed is non-zero, that indicate we are support depth color mode(but I think the depth color mode need at least 36bpp). Besides as the HDMI Specification descripte, "/If the Sink dose not receive a GCP with non-zero CD from more than 4 consecutive video// //fields, it should exit deep color mode and reverting to 24-bit color/"(24-bit color is default). In the end I think if we only output 24-bit color, we should make the CD field to zero :) > Are you saying that you wish to pass something other than 24 bpp video > to your HDMI encoder? > >> In the end we should keep the *csc_color_depth(HDMI_CSC_SCALE)* & >> *color_depth(HDMI_VP_PR_CD)* to zero, code should modify like this GCP >> would test pass: > >From what you're describing, you want CD field = 0 and CSC_SCALE = 0. > It looks like hdmi_video_packetize() will set the CD field to zero if > hdmi_data.enc_color_depth = 0, but that would cause hdmi_video_sample() > and hdmi_video_csc() to fail. Maybe those two functions should be fixed > to accept a color depth of zero, and maybe you need to set > enc_color_depth to 0? > > Interestingly, HDMI_CSC_SCALE_CSC_COLORDE_PTH_24BPP is defined to be > zero, but again, in the iMX6 data, it could take a value of either > 0x00 or 0x40. I think hdmi_video_csc() should set this to 0x40 if > hdmi_data.enc_color_depth = 0, and 0x40 for hdmi_data.enc_color_depth = 8. Agree! If we only output 24-bit color, we should let the hdmi_data.enc_color_depth = 0, and hdmi_video_csc() & hdmi_video_packetize() should handle the "enc_color_depth = 0". Best regards. Yakir Yang
diff --git a/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c index bf5be679..c5d9b16 100644 --- a/drivers/gpu/drm/bridge/dw_hdmi.c +++ b/drivers/gpu/drm/bridge/dw_hdmi.c @@ -695,7 +695,7 @@ static void hdmi_video_packetize(struct dw_hdmi *hdmi) if (!hdmi_data->enc_color_depth) { output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS; } else if (hdmi_data->enc_color_depth == 8) { - color_depth = 4; + color_depth = 0; output_select = HDMI_VP_CONF_OUTPUT_SELECTOR_BYPASS;