On Thu, Jul 13, 2017 at 06:40:36PM +0530, Sharma, Shashank wrote:
> Regards
>
> Shashank
>
>
> On 7/13/2017 6:25 PM, Ville Syrjälä wrote:
> > On Thu, Jul 13, 2017 at 06:09:53PM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 7/13/2017 5:57 PM, Ville Syrjälä wrote:
> >>> On Thu, Jul 13, 2017 at 11:11:53AM +0530, Sharma, Shashank wrote:
> >>>> Regards
> >>>>
> >>>> Shashank
> >>>>
> >>>>
> >>>> On 7/12/2017 10:54 PM, Ville Syrjälä wrote:
> >>>>> On Mon, Jul 10, 2017 at 04:48:48PM +0530, Shashank Sharma wrote:
> >>>>>> LSPCON chips can't pick the HDMI AVI infoframes from direct link.
> >>>>>> In order to pass AVI infoframes from display controller to LSPCON,
> >>>>>> we have to write infoframe packets into vendor specified AUX address.
> >>>>>>
> >>>>>> Also, LSPCON vendors provide AUX offsets, to inform the LSPCON
> >>>>>> chip that the AVI IF packets are written, so that the firmware
> >>>>>> can pick it up and apply.
> >>>>>>
> >>>>>> This patch adds function to write AVI infoframes for both MCA as
> >>>>>> well as Parade Tech LSPCON chips. These two vendors provide different
> >>>>>> methods for writing infoframes, so this patch contains two different
> >>>>>> functions, one for each.
> >>>>> Seems to me that we should also be checking the receiver cap for the
> >>>>> 444->420 conversion support, and enable it in the PROTOCOL_CONVERTER
> >>>>> DPCD register. Or are these LSPCON things even ignoring that part of the
> >>>>> spec?
> >>>> Yes, LSPCON just needs the YCBCR444 input, and info frames for 420, and
> >>>> it would take care of
> >>>> 444->420 conversion in FW.
> >>> So it snoops the AVI infoframe we write to it to figure out that it has
> >>> to do the 444->420 downsampling?
> >> Yes, that's exactly what it does.
> > Can you add a comment somewhere appropriate to make that clear.
> > Otherwise people might wonder why we're not enabling the downsampling at
> > all.
> >
> > Does LSPCON then not even implement the PROTOCOL_CONVERTER DCPD
> > registers?
> >
> > Can you send the spec for this stuff my way? What I have now doesn't
> > document any DPCD registers.
> Actually I also don't have a document/spec which describes this. This is
> kind of knowledge sharing from people who
> had design discussions with MCA/PARADE vendors on how to implement
> LSPCON specs and features.
So there are no specs at all? That's no way to do things. As is, no
one can actually review this code without experimenting with the
hardware themselves.
> Typically LSPCON firmware parses the AVI IF packets, and does the
> scaling down from 444->420. That's why
> we are writing the AVI IF at custom AUX location.
>
> I have added this description in patch 18's text, which talks about the
> steps to get a YCBCR420 output from LSPCON.
> It says:
>
> LSPCON chips support YCBCR420 outputs. To be able to get
> YCBCR420 output from LSPCON chip, the source should:
> - Generate YCBCR444 HDMI output
> - Set AVI infoframes for a YCBCR420 output.
>
> And added this small comment in 420_config:
> + /* LSPCON doesn't need scaler for YCBCR420 output */
> + if (config->lspcon_active)
> + return true;
> +
>
> But probably I should discreetly mention that LSPCON doesn't need a scalar somehow.
Yeah something like this perhaps:
"We send 4:4:4 data to LSPCON which performs the 4:4:4->4:2:0
downsampling or us, hence we don't need a pipe scaler."
>
> - Shashank
>
> >> - Shashank
> >>>> Also, its a part of LSPCON specs, that it has
> >>>> to have 420 output capability.
> >>>>
> >>>> - Shashank
> >>>>>> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> >>>>>> Cc: Imre Deak <imre.deak@xxxxxxxxx>
> >>>>>> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> >>>>>> ---
> >>>>>> drivers/gpu/drm/i915/intel_ddi.c | 8 ++
> >>>>>> drivers/gpu/drm/i915/intel_drv.h | 3 +
> >>>>>> drivers/gpu/drm/i915/intel_lspcon.c | 174 ++++++++++++++++++++++++++++++++++++
> >>>>>> 3 files changed, 185 insertions(+)
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >>>>>> index f691710..944d9d5 100644
> >>>>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >>>>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >>>>>> @@ -2068,6 +2068,14 @@ void intel_ddi_set_avi_infoframe(struct drm_encoder *encoder,
> >>>>>> rgb_qrange_limited,
> >>>>>> rgb_qrange_selectable);
> >>>>>>
> >>>>>> + if (crtc_state->lspcon_active) {
> >>>>>> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
> >>>>>> +
> >>>>>> + /* LSPCON writes infoframes via AUX */
> >>>>>> + lspcon->write_infoframe(encoder, crtc_state, &frame);
> >>>>>> + return;
> >>>>>> + }
> >>>>>> +
> >>>>>> intel_write_infoframe(encoder, crtc_state, &frame);
> >>>>>> }
> >>>>>>
> >>>>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>>>>> index fad9a53..3e686d2 100644
> >>>>>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>>>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>>>>> @@ -1079,6 +1079,9 @@ struct intel_lspcon {
> >>>>>> /* AVI IF setup function for LSPCON */
> >>>>>> void (*set_infoframes)(struct drm_encoder *encoder,
> >>>>>> const struct intel_crtc_state *crtc_state);
> >>>>>> + void (*write_infoframe)(struct drm_encoder *encoder,
> >>>>>> + const struct intel_crtc_state *crtc_state,
> >>>>>> + union hdmi_infoframe *frame);
> >>>>>> };
> >>>>>>
> >>>>>> struct intel_digital_port {
> >>>>>> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> >>>>>> index 53ddd39..01fddf7 100644
> >>>>>> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> >>>>>> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> >>>>>> @@ -31,6 +31,18 @@
> >>>>>> #define LSPCON_VENDOR_PARADE_OUI 0x001CF8
> >>>>>> #define LSPCON_VENDOR_MCA_OUI 0x0060AD
> >>>>>>
> >>>>>> +/* AUX addresses to write AVI IF into */
> >>>>>> +#define LSPCON_MCA_AVI_IF_WRITE_OFFSET 0x5C0
> >>>>>> +#define LSPCON_MCA_AVI_IF_CTRL 0x5DF
> >>>>>> +#define LSPCON_MCA_AVI_IF_KICKOFF (1 << 0)
> >>>>>> +#define LSPCON_MCA_AVI_IF_HANDLED (1 << 1)
> >>>>>> +
> >>>>>> +#define LSPCON_PARADE_AVI_IF_WRITE_OFFSET 0x516
> >>>>>> +#define LSPCON_PARADE_AVI_IF_CTRL 0x51E
> >>>>>> +#define LSPCON_PARADE_AVI_IF_KICKOFF (1 << 7)
> >>>>>> +#define LSPCON_PARADE_AVI_IF_STATUS 0x51F
> >>>>>> +#define LSPCON_PARADE_AVI_IF_HANDLED (2 << 6)
> >>>>>> +
> >>>>>> static struct intel_dp *lspcon_to_intel_dp(struct intel_lspcon *lspcon)
> >>>>>> {
> >>>>>> struct intel_digital_port *dig_port =
> >>>>>> @@ -217,6 +229,167 @@ bool lspcon_ycbcr420_config(struct drm_connector *connector,
> >>>>>> return false;
> >>>>>> }
> >>>>>>
> >>>>>> +static bool _lspcon_write_infoframe_parade(struct drm_dp_aux *aux,
> >>>>>> + uint8_t *buffer, ssize_t len)
> >>>>>> +{
> >>>>>> + u8 avi_if_ctrl;
> >>>>>> + u8 avi_if_status;
> >>>>>> + u8 count = 0;
> >>>>>> + u8 retry = 5;
> >>>>>> + u8 avi_buf[8] = {0, };
> >>>>>> + uint16_t reg;
> >>>>>> + ssize_t ret;
> >>>>>> +
> >>>>>> + while (count++ < 4) {
> >>>>>> +
> >>>>>> + do {
> >>>>>> + /* Is LSPCON FW ready */
> >>>>>> + reg = LSPCON_PARADE_AVI_IF_CTRL;
> >>>>>> + ret = drm_dp_dpcd_read(aux, reg, &avi_if_ctrl, 1);
> >>>>>> + if (ret < 0) {
> >>>>>> + DRM_ERROR("DPCD read failed, add:0x%x\n", reg);
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)
> >>>>>> + break;
> >>>>>> + usleep_range(100, 200);
> >>>>>> + } while (--retry);
> >>>>>> +
> >>>>>> + if (!(avi_if_ctrl & LSPCON_PARADE_AVI_IF_KICKOFF)) {
> >>>>>> + DRM_ERROR("LSPCON FW not ready for infoframes\n");
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> +
> >>>>>> + /*
> >>>>>> + * AVI Infoframe contains 31 bytes of data:
> >>>>>> + * HB0 to HB2 (3 bytes header)
> >>>>>> + * PB0 to PB27 (28 bytes data)
> >>>>>> + * As per Parade spec, while sending first block (8bytes),
> >>>>>> + * byte 0 is kept for request token no, and byte1 - byte7
> >>>>>> + * contain frame data. So we have to pack frame like this:
> >>>>>> + * first block of 8 bytes: <token> <HB0-HB2> <PB0-PB3>
> >>>>>> + * next 3 blocks: <PB4-PB27>
> >>>>>> + */
> >>>>>> + if (count)
> >>>>>> + memcpy(avi_buf, buffer + count * 8 - 1, 8);
> >>>>>> + else {
> >>>>>> + avi_buf[0] = 1;
> >>>>>> + memcpy(&avi_buf[1], buffer, 7);
> >>>>>> + }
> >>>>>> +
> >>>>>> + /* Write 8 bytes of data at a time */
> >>>>>> + reg = LSPCON_PARADE_AVI_IF_WRITE_OFFSET;
> >>>>>> + ret = drm_dp_dpcd_write(aux, reg, avi_buf, 8);
> >>>>>> + if (ret < 0) {
> >>>>>> + DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> +
> >>>>>> + /*
> >>>>>> + * While sending a block of 8 byes, we need to inform block
> >>>>>> + * number to FW, by programming bits[1:0] of ctrl reg with
> >>>>>> + * block number
> >>>>>> + */
> >>>>>> + avi_if_ctrl = 0x80 + count;
> >>>>>> + reg = LSPCON_PARADE_AVI_IF_CTRL;
> >>>>>> + ret = drm_dp_dpcd_write(aux, reg, &avi_if_ctrl, 1);
> >>>>>> + if (ret < 0) {
> >>>>>> + DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> + }
> >>>>>> +
> >>>>>> + /* Check LSPCON FW status */
> >>>>>> + reg = LSPCON_PARADE_AVI_IF_STATUS;
> >>>>>> + ret = drm_dp_dpcd_read(aux, reg, &avi_if_status, 1);
> >>>>>> + if (ret < 0) {
> >>>>>> + DRM_ERROR("DPCD write failed, address 0x%x\n", reg);
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (avi_if_status & LSPCON_PARADE_AVI_IF_HANDLED)
> >>>>>> + DRM_DEBUG_KMS("AVI IF handled by FW\n");
> >>>>>> +
> >>>>>> + return true;
> >>>>>> +}
> >>>>>> +
> >>>>>> +static bool _lspcon_write_infoframe_mca(struct drm_dp_aux *aux,
> >>>>>> + uint8_t *buffer, ssize_t len)
> >>>>>> +{
> >>>>>> + int ret;
> >>>>>> + uint32_t val = 0;
> >>>>>> + uint16_t reg;
> >>>>>> + uint8_t *data = buffer;
> >>>>>> +
> >>>>>> + reg = LSPCON_MCA_AVI_IF_WRITE_OFFSET;
> >>>>>> + while (val < len) {
> >>>>>> + ret = drm_dp_dpcd_write(aux, reg, data, 1);
> >>>>>> + if (ret < 0) {
> >>>>>> + DRM_ERROR("DPCD write failed, add:0x%x\n", reg);
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> + val++; reg++; data++;
> >>>>>> + }
> >>>>>> +
> >>>>>> + val = 0;
> >>>>>> + reg = LSPCON_MCA_AVI_IF_CTRL;
> >>>>>> + ret = drm_dp_dpcd_read(aux, reg, &val, 1);
> >>>>>> + if (ret < 0) {
> >>>>>> + DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> +
> >>>>>> + /* Indicate LSPCON chip about infoframe, clear bit 1 and set bit 0 */
> >>>>>> + val &= ~LSPCON_MCA_AVI_IF_HANDLED;
> >>>>>> + val |= LSPCON_MCA_AVI_IF_KICKOFF;
> >>>>>> +
> >>>>>> + ret = drm_dp_dpcd_write(aux, reg, &val, 1);
> >>>>>> + if (ret < 0) {
> >>>>>> + DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> +
> >>>>>> + val = 0;
> >>>>>> + ret = drm_dp_dpcd_read(aux, reg, &val, 1);
> >>>>>> + if (ret < 0) {
> >>>>>> + DRM_ERROR("DPCD read failed, address 0x%x\n", reg);
> >>>>>> + return false;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (val == LSPCON_MCA_AVI_IF_HANDLED)
> >>>>>> + DRM_DEBUG_KMS("AVI IF handled by FW\n");
> >>>>>> +
> >>>>>> + return true;
> >>>>>> +}
> >>>>>> +
> >>>>>> +void lspcon_write_infoframe(struct drm_encoder *encoder,
> >>>>>> + const struct intel_crtc_state *crtc_state,
> >>>>>> + union hdmi_infoframe *frame)
> >>>>>> +{
> >>>>>> + bool ret;
> >>>>>> + ssize_t len;
> >>>>>> + uint8_t buf[VIDEO_DIP_DATA_SIZE];
> >>>>>> + struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> >>>>>> + struct intel_lspcon *lspcon = enc_to_intel_lspcon(encoder);
> >>>>>> +
> >>>>>> + len = hdmi_infoframe_pack(frame, buf, sizeof(buf));
> >>>>>> + if (len < 0) {
> >>>>>> + DRM_ERROR("Failed to pack AVI IF\n");
> >>>>>> + return;
> >>>>>> + }
> >>>>>> +
> >>>>>> + if (lspcon->vendor == LSPCON_VENDOR_MCA)
> >>>>>> + ret = _lspcon_write_infoframe_mca(&intel_dp->aux, buf, len);
> >>>>>> + else
> >>>>>> + ret = _lspcon_write_infoframe_parade(&intel_dp->aux, buf, len);
> >>>>>> +
> >>>>>> + if (!ret)
> >>>>>> + DRM_ERROR("Failed to write AVI infoframes\n");
> >>>>>> + else
> >>>>>> + DRM_DEBUG_DRIVER("AVI infoframes updated successfully\n");
> >>>>>> +}
> >>>>>> +
> >>>>>> void lspcon_resume(struct intel_lspcon *lspcon)
> >>>>>> {
> >>>>>> enum drm_lspcon_mode expected_mode;
> >>>>>> @@ -282,6 +455,7 @@ bool lspcon_init(struct intel_digital_port *intel_dig_port)
> >>>>>>
> >>>>>> connector->ycbcr_420_allowed = true;
> >>>>>> lspcon->set_infoframes = intel_ddi_set_avi_infoframe;
> >>>>>> + lspcon->write_infoframe = lspcon_write_infoframe;
> >>>>>> drm_dp_read_desc(&dp->aux, &dp->desc, drm_dp_is_branch(dp->dpcd));
> >>>>>>
> >>>>>> DRM_DEBUG_KMS("Success: LSPCON init\n");
> >>>>>> --
> >>>>>> 2.7.4
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
