Hi, Some comments below... On Mon, Jan 18, 2016 at 10:42:20PM +0800, Yakir Yang wrote: > +static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi) > +{ > + char info[HDMI_SIZE_AVI_INFOFRAME] = {0}; > + int avi_color_mode; > + int i; > + > + hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_BUF_INDEX, INFOFRAME_AVI); > + > + info[0] = 0x82; > + info[1] = 0x02; > + info[2] = 0x0D; > + info[3] = info[0] + info[1] + info[2]; > + > + if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_RGB) > + avi_color_mode = AVI_COLOR_MODE_RGB; > + else if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444) > + avi_color_mode = AVI_COLOR_MODE_YCBCR444; > + else if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV422) > + avi_color_mode = AVI_COLOR_MODE_YCBCR422; > + else > + avi_color_mode = AVI_COLOR_MODE_RGB; > + > + info[4] = (avi_color_mode << 5); > + info[5] = (AVI_COLORIMETRY_NO_DATA << 6) | > + (AVI_CODED_FRAME_ASPECT_NO_DATA << 4) | > + ACTIVE_ASPECT_RATE_SAME_AS_CODED_FRAME; > + > + info[6] = 0; > + info[7] = hdmi->hdmi_data.vic; > + > + if (hdmi->hdmi_data.vic == 6 || hdmi->hdmi_data.vic == 7 || > + hdmi->hdmi_data.vic == 21 || hdmi->hdmi_data.vic == 22) > + info[8] = 1; > + else > + info[8] = 0; > + > + /* Calculate avi info frame checKsum */ > + for (i = 4; i < HDMI_SIZE_AVI_INFOFRAME; i++) > + info[3] += info[i]; > + info[3] = 0x100 - info[3]; > + > + for (i = 0; i < HDMI_SIZE_AVI_INFOFRAME; i++) > + hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_ADDR + i, info[i]); > + > + return 0; Is there a reason why the helpers in drivers/video/hdmi.c can't be used to generate this? hdmi_avi_infoframe_init / hdmi_avi_infoframe_pack and drm_hdmi_avi_infoframe_from_display_mode ? > +} > + > +static int inno_hdmi_config_video_vsi(struct inno_hdmi *hdmi) > +{ > + char info[HDMI_SIZE_VSI_INFOFRAME] = {0}; > + int i; > + > + hdmi_modb(hdmi, HDMI_PACKET_SEND_AUTO, m_PACKET_VSI_EN, > + v_PACKET_VSI_EN(0)); > + > + hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_BUF_INDEX, INFOFRAME_VSI); > + > + /* Header Bytes */ > + info[0] = 0x81; > + info[1] = 0x01; > + > + /* PB1 - PB3 contain the 24bit IEEE Registration Identifier */ > + info[4] = 0x03; > + info[5] = 0x0c; > + info[6] = 0x00; > + > + /* PB4 - HDMI_Video_Format into bits 7:5 */ > + info[7] = 0; > + > + /* > + * PB5 - Depending on the video format, this byte will contain > + * either the HDMI_VIC code in buts 7:0, OR the 3D_Structure in > + * bits 7:4. > + */ > + info[2] = 0x06 - 2; > + info[8] = 0; > + info[9] = 0; > + > + info[3] = info[0] + info[1] + info[2]; > + > + /* Calculate info frame checKsum */ > + for (i = 4; i < HDMI_SIZE_VSI_INFOFRAME; i++) > + info[3] += info[i]; > + info[3] = 0x100 - info[3]; > + > + for (i = 0; i < HDMI_SIZE_VSI_INFOFRAME; i++) > + hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_ADDR + i, info[i]); > + > + hdmi_modb(hdmi, HDMI_PACKET_SEND_AUTO, m_PACKET_VSI_EN, > + v_PACKET_VSI_EN(1)); > + > + return 0; hdmi_vendor_infoframe_init / hdmi_vendor_infoframe_pack? You can probably use the same function to upload the control packet too - something like: static int inno_hdmi_upload_frame(struct inno_hdmi *hdmi, int setup_rc, union hdmi_infoframe *frame, u32 mask, u32 disable, u32 enable) { if (mask) hdmi_modb(hdmi, HDMI_PACKET_SEND_AUTO, mask, disable); if (setup_rc >= 0) { u8 packed_frame[YOUR_MAXIMUM_INFO_FRAME_SIZE]; ssize_t rc, i; rc = hdmi_infoframe_pack(frame, packed_frame, sizeof(packed_frame)); if (rc < 0) return rc; for (i = 0; i < rc; i++) hdmi_writeb(hdmi, HDMI_CONTROL_PACKET_ADDR + i, buf[i]); if (mask) hdmi_modb(hdmi, HDMI_PACKET_SEND_AUTO, mask, enable); } return setup_rc; } static int inno_hdmi_config_video_vsi(struct inno_hdmi *hdmi) { union hdmi_infoframe frame; rc = drm_hdmi_vendor_infoframe_from_display_mode(&frame.vendor.hdmi, the_drm_display_mode); return inno_hdmi_upload_frame(hdmi, rc, &frame, m_PACKET_VSI_EN, v_PACKET_VSI_EN(0), v_PACKET_VSI_EN(1)); } > +static int inno_hdmi_i2c_wait(struct inno_hdmi *hdmi) > +{ > + struct inno_hdmi_i2c *i2c = hdmi->i2c; > + int stat; > + > + stat = wait_for_completion_timeout(&i2c->cmp, HZ / 10); > + if (!stat) { > + stat = wait_for_completion_timeout(&i2c->cmp, HZ / 10); > + if (!stat) > + return -EAGAIN; What's the reason for this double wait_for_completion? This probably needs a comment, otherwise it looks like some weird cut-n-paste error. > +static int inno_hdmi_i2c_write(struct inno_hdmi *hdmi, struct i2c_msg *msgs) > +{ > + struct inno_hdmi_i2c *i2c = hdmi->i2c; You seem to have a local i2c pointer... > + > + /* > + * The DDC module only support read EDID message, so > + * we assume that each word write to this i2c adapter > + * should be the offset of EDID word address. > + */ > + if ((msgs->len != 1) || > + ((msgs->addr != DDC_ADDR) && (msgs->addr != DDC_SEGMENT_ADDR))) > + return -EINVAL; > + > + reinit_completion(&i2c->cmp); > + > + if (msgs->addr == DDC_SEGMENT_ADDR) > + hdmi->i2c->segment_addr = msgs->buf[0]; > + if (msgs->addr == DDC_ADDR) > + hdmi->i2c->ddc_addr = msgs->buf[0]; > + > + /* Set edid fifo first addr */ > + hdmi_writeb(hdmi, HDMI_EDID_FIFO_OFFSET, 0x00); > + > + /* Set edid word address 0x00/0x80 */ > + hdmi_writeb(hdmi, HDMI_EDID_WORD_ADDR, hdmi->i2c->ddc_addr); > + > + /* Set edid segment pointer */ > + hdmi_writeb(hdmi, HDMI_EDID_SEGMENT_POINTER, hdmi->i2c->segment_addr); but then you don't use it in the four locations above. -- RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html