From: Daniel Vetter <daniel@ffwll.ch>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 14/18] drm/i915: Read out HDMI infoframes
Date: Mon, 24 Sep 2018 18:08:09 +0200 [thread overview]
Message-ID: <20180924160809.GA11082@phenom.ffwll.local> (raw)
In-Reply-To: <20180920185145.1912-15-ville.syrjala@linux.intel.com>
On Thu, Sep 20, 2018 at 09:51:41PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add code to read the infoframes from the video DIP and unpack them into
> the crtc state.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_ddi.c | 17 ++++
> drivers/gpu/drm/i915/intel_drv.h | 10 ++
> drivers/gpu/drm/i915/intel_hdmi.c | 203 ++++++++++++++++++++++++++++++++++++++
> 3 files changed, 230 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 5f3bd536d261..a56289f78326 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -3459,6 +3459,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
>
> intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
> +
> + intel_hdmi_read_gcp_infoframe(encoder, pipe_config);
> +
> + if (!intel_read_infoframe(encoder, pipe_config,
> + HDMI_INFOFRAME_TYPE_AVI,
> + &pipe_config->infoframes.avi))
> + DRM_ERROR("failed to read AVI infoframe\n");
> +
> + if (!intel_read_infoframe(encoder, pipe_config,
> + HDMI_INFOFRAME_TYPE_SPD,
> + &pipe_config->infoframes.spd))
> + DRM_ERROR("failed to read SPD infoframe:\n");
> +
> + if (!intel_read_infoframe(encoder, pipe_config,
> + HDMI_INFOFRAME_TYPE_VENDOR,
> + &pipe_config->infoframes.hdmi))
> + DRM_ERROR("failed to read HDMI infoframe\n");
> }
>
> static enum intel_output_type
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 357624a6bfe2..75ec99b85232 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1185,6 +1185,10 @@ struct intel_digital_port {
> const struct intel_crtc_state *crtc_state,
> unsigned int type,
> const void *frame, ssize_t len);
> + ssize_t (*read_infoframe)(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + unsigned int type,
> + void *frame, ssize_t len);
> void (*set_infoframes)(struct intel_encoder *encoder,
> bool enable,
> const struct intel_crtc_state *crtc_state,
> @@ -1867,6 +1871,12 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port);
> u32 intel_hdmi_infoframes_enabled(struct intel_encoder *encoder,
> const struct intel_crtc_state *crtc_state);
> u32 intel_hdmi_infoframe_enable(unsigned int type);
> +void intel_hdmi_read_gcp_infoframe(struct intel_encoder *encoder,
> + struct intel_crtc_state *crtc_state);
> +bool intel_read_infoframe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + enum hdmi_infoframe_type type,
> + union hdmi_infoframe *frame);
>
>
> /* intel_lvds.c */
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 491001fc0fad..27cb6ec32e94 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -203,6 +203,31 @@ static void g4x_write_infoframe(struct intel_encoder *encoder,
> POSTING_READ(VIDEO_DIP_CTL);
> }
>
> +static ssize_t g4x_read_infoframe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + unsigned int type,
> + void *frame, ssize_t len)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + u32 val, *data = frame;
> + int i;
> +
> + val = I915_READ(VIDEO_DIP_CTL);
> +
> + if ((val & g4x_infoframe_enable(type)) == 0)
> + return 0;
> +
> + val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
> + val |= g4x_infoframe_index(type);
> +
> + I915_WRITE(VIDEO_DIP_CTL, val);
> +
> + for (i = 0; i < len; i += 4)
> + *data++ = I915_READ(VIDEO_DIP_DATA);
> +
> + return len;
> +}
> +
> static u32 g4x_infoframes_enabled(struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config)
> {
> @@ -258,6 +283,32 @@ static void ibx_write_infoframe(struct intel_encoder *encoder,
> POSTING_READ(reg);
> }
>
> +static ssize_t ibx_read_infoframe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + unsigned int type,
> + void *frame, ssize_t len)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + u32 val, *data = frame;
> + int i;
> +
> + val = I915_READ(TVIDEO_DIP_CTL(crtc->pipe));
> +
> + if ((val & g4x_infoframe_enable(type)) == 0)
> + return 0;
> +
> + val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
> + val |= g4x_infoframe_index(type);
> +
> + I915_WRITE(TVIDEO_DIP_CTL(crtc->pipe), val);
> +
> + for (i = 0; i < len; i += 4)
> + *data++ = I915_READ(TVIDEO_DIP_DATA(crtc->pipe));
> +
> + return len;
> +}
> +
> static u32 ibx_infoframes_enabled(struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config)
> {
> @@ -319,6 +370,32 @@ static void cpt_write_infoframe(struct intel_encoder *encoder,
> POSTING_READ(reg);
> }
>
> +static ssize_t cpt_read_infoframe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + unsigned int type,
> + void *frame, ssize_t len)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + u32 val, *data = frame;
> + int i;
> +
> + val = I915_READ(TVIDEO_DIP_CTL(crtc->pipe));
> +
> + if ((val & g4x_infoframe_enable(type)) == 0)
> + return 0;
> +
> + val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
> + val |= g4x_infoframe_index(type);
> +
> + I915_WRITE(TVIDEO_DIP_CTL(crtc->pipe), val);
> +
> + for (i = 0; i < len; i += 4)
> + *data++ = I915_READ(TVIDEO_DIP_DATA(crtc->pipe));
> +
> + return len;
> +}
> +
> static u32 cpt_infoframes_enabled(struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config)
> {
> @@ -373,6 +450,32 @@ static void vlv_write_infoframe(struct intel_encoder *encoder,
> POSTING_READ(reg);
> }
>
> +static ssize_t vlv_read_infoframe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + unsigned int type,
> + void *frame, ssize_t len)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + u32 val, *data = frame;
> + int i;
> +
> + val = I915_READ(VLV_TVIDEO_DIP_CTL(crtc->pipe));
> +
> + if ((val & g4x_infoframe_enable(type)) == 0)
> + return 0;
> +
> + val &= ~(VIDEO_DIP_SELECT_MASK | 0xf); /* clear DIP data offset */
> + val |= g4x_infoframe_index(type);
> +
> + I915_WRITE(VLV_TVIDEO_DIP_CTL(crtc->pipe), val);
> +
> + for (i = 0; i < len; i += 4)
> + *data++ = I915_READ(VLV_TVIDEO_DIP_DATA(crtc->pipe));
> +
> + return len;
> +}
> +
> static u32 vlv_infoframes_enabled(struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config)
> {
> @@ -425,6 +528,28 @@ static void hsw_write_infoframe(struct intel_encoder *encoder,
> POSTING_READ(ctl_reg);
> }
>
> +static ssize_t hsw_read_infoframe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + unsigned int type,
> + void *frame, ssize_t len)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> + u32 val, *data = frame;
> + int i;
> +
> + val = I915_READ(HSW_TVIDEO_DIP_CTL(cpu_transcoder));
> +
> + if ((val & hsw_infoframe_enable(type)) == 0)
> + return 0;
> +
> + for (i = 0; i < len; i += 4)
> + *data++ = I915_READ(hsw_dip_data_reg(dev_priv, cpu_transcoder,
> + type, i >> 2));
> +
> + return len;
> +}
> +
> static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
> const struct intel_crtc_state *pipe_config)
> {
> @@ -530,6 +655,39 @@ static void intel_write_infoframe(struct intel_encoder *encoder,
> intel_dig_port->write_infoframe(encoder, crtc_state, type, buffer, len);
> }
>
> +bool intel_read_infoframe(struct intel_encoder *encoder,
> + const struct intel_crtc_state *crtc_state,
> + enum hdmi_infoframe_type type,
> + union hdmi_infoframe *frame)
Bit a bikeshed: I'd drop the boolean here and pull the debug output in.
That way you can give a bit better hint about what's going wrong. And less
duplicated code. You can still print the type.
> +{
> + struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base);
> + u8 buffer[VIDEO_DIP_DATA_SIZE];
> + ssize_t len;
> + int ret;
> +
> + if ((crtc_state->infoframes.enable &
> + intel_hdmi_infoframe_enable(type)) == 0)
> + return true;
Afaiui you only need this because g4x doesn't check the pipe (well, port)
association. But then all the tests once again check this by confirming
that the infoframe they should read out is enabled. I'd drop the check in
the various hw-specific readout functions (it's duplicated). That should
still work with g4x here, as long as you filter at this level here.
With or without the bikesheds:
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> +
> + len = intel_dig_port->read_infoframe(encoder, crtc_state,
> + type, buffer, sizeof(buffer));
> + if (len == 0)
> + return true;
> +
> + /* Fill the 'hole' (see big comment above) at position 3 */
> + memmove(&buffer[1], &buffer[0], 3);
> +
> + /* see comment above for the reason for this offset */
> + ret = hdmi_infoframe_unpack(frame, buffer + 1, sizeof(buffer) - 1);
> + if (ret)
> + return false;
> +
> + if (frame->any.type != type)
> + return false;
> +
> + return true;
> +}
> +
> static bool
> intel_hdmi_compute_avi_infoframe(struct intel_encoder *encoder,
> struct intel_crtc_state *crtc_state,
> @@ -784,6 +942,29 @@ static bool intel_hdmi_set_gcp_infoframe(struct intel_encoder *encoder,
> return true;
> }
>
> +void intel_hdmi_read_gcp_infoframe(struct intel_encoder *encoder,
> + struct intel_crtc_state *crtc_state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> + i915_reg_t reg;
> +
> + if ((crtc_state->infoframes.enable &
> + intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GENERAL_CONTROL)) == 0)
> + return;
> +
> + if (HAS_DDI(dev_priv))
> + reg = HSW_TVIDEO_DIP_GCP(crtc_state->cpu_transcoder);
> + else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> + reg = VLV_TVIDEO_DIP_GCP(crtc->pipe);
> + else if (HAS_PCH_SPLIT(dev_priv))
> + reg = TVIDEO_DIP_GCP(crtc->pipe);
> + else
> + return;
> +
> + crtc_state->infoframes.gcp = I915_READ(reg);
> +}
> +
> static void intel_hdmi_compute_gcp_infoframe(struct intel_encoder *encoder,
> struct intel_crtc_state *crtc_state,
> struct drm_connector_state *conn_state)
> @@ -1382,6 +1563,23 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
> pipe_config->base.adjusted_mode.crtc_clock = dotclock;
>
> pipe_config->lane_count = 4;
> +
> + intel_hdmi_read_gcp_infoframe(encoder, pipe_config);
> +
> + if (!intel_read_infoframe(encoder, pipe_config,
> + HDMI_INFOFRAME_TYPE_AVI,
> + &pipe_config->infoframes.avi))
> + DRM_ERROR("failed to read AVI infoframe\n");
> +
> + if (!intel_read_infoframe(encoder, pipe_config,
> + HDMI_INFOFRAME_TYPE_SPD,
> + &pipe_config->infoframes.spd))
> + DRM_ERROR("failed to read SPD infoframe:\n");
> +
> + if (!intel_read_infoframe(encoder, pipe_config,
> + HDMI_INFOFRAME_TYPE_VENDOR,
> + &pipe_config->infoframes.hdmi))
> + DRM_ERROR("failed to read HDMI infoframe\n");
> }
>
> static void intel_enable_hdmi_audio(struct intel_encoder *encoder,
> @@ -2481,22 +2679,27 @@ void intel_infoframe_init(struct intel_digital_port *intel_dig_port)
>
> if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
> intel_dig_port->write_infoframe = vlv_write_infoframe;
> + intel_dig_port->read_infoframe = vlv_read_infoframe;
> intel_dig_port->set_infoframes = vlv_set_infoframes;
> intel_dig_port->infoframes_enabled = vlv_infoframes_enabled;
> } else if (IS_G4X(dev_priv)) {
> intel_dig_port->write_infoframe = g4x_write_infoframe;
> + intel_dig_port->read_infoframe = g4x_read_infoframe;
> intel_dig_port->set_infoframes = g4x_set_infoframes;
> intel_dig_port->infoframes_enabled = g4x_infoframes_enabled;
> } else if (HAS_DDI(dev_priv)) {
> intel_dig_port->write_infoframe = hsw_write_infoframe;
> + intel_dig_port->read_infoframe = hsw_read_infoframe;
> intel_dig_port->set_infoframes = hsw_set_infoframes;
> intel_dig_port->infoframes_enabled = hsw_infoframes_enabled;
> } else if (HAS_PCH_IBX(dev_priv)) {
> intel_dig_port->write_infoframe = ibx_write_infoframe;
> + intel_dig_port->read_infoframe = ibx_read_infoframe;
> intel_dig_port->set_infoframes = ibx_set_infoframes;
> intel_dig_port->infoframes_enabled = ibx_infoframes_enabled;
> } else {
> intel_dig_port->write_infoframe = cpt_write_infoframe;
> + intel_dig_port->read_infoframe = cpt_read_infoframe;
> intel_dig_port->set_infoframes = cpt_set_infoframes;
> intel_dig_port->infoframes_enabled = cpt_infoframes_enabled;
> }
> --
> 2.16.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
next prev parent reply other threads:[~2018-09-24 16:08 UTC|newest]
Thread overview: 69+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-20 18:51 [PATCH 00/18] drm/i915: Infoframe precompute/check Ville Syrjala
2018-09-20 18:51 ` [PATCH 01/18] video/hdmi: Constify 'buffer' to the unpack functions Ville Syrjala
2018-09-21 8:03 ` Hans Verkuil
2018-09-20 18:51 ` [PATCH 02/18] video/hdmi: Pass buffer size to infoframe " Ville Syrjala
2018-09-21 8:06 ` Hans Verkuil
2018-09-20 18:51 ` [PATCH 03/18] video/hdmi: Constify infoframe passed to the log functions Ville Syrjala
2018-09-21 8:06 ` Hans Verkuil
2018-09-20 18:51 ` [PATCH 04/18] video/hdmi: Constify infoframe passed to the pack functions Ville Syrjala
2018-09-21 8:24 ` Hans Verkuil
2018-09-21 14:30 ` Ville Syrjälä
2018-09-21 14:33 ` [PATCH v3 " Ville Syrjala
2018-10-01 19:10 ` Ville Syrjälä
2018-10-02 6:37 ` Hans Verkuil
2018-09-20 18:51 ` [PATCH 05/18] video/hdmi: Add an enum for HDMI packet types Ville Syrjala
2018-09-21 8:41 ` Hans Verkuil
2018-09-21 14:01 ` Ville Syrjälä
2018-09-21 14:12 ` Hans Verkuil
2018-09-21 15:07 ` Ville Syrjälä
2018-12-20 11:27 ` Sharma, Shashank
2018-09-20 18:51 ` [PATCH 06/18] video/hdmi: Handle the MPEG Source infoframe Ville Syrjala
2018-09-21 8:28 ` Hans Verkuil
2018-09-21 13:53 ` Ville Syrjälä
2018-09-21 15:09 ` [PATCH v2 " Ville Syrjala
2018-09-20 18:51 ` [PATCH 07/18] video/hdmi: Handle the NTSC VBI infoframe Ville Syrjala
2018-09-21 8:30 ` Hans Verkuil
2018-09-21 13:54 ` Ville Syrjälä
2018-09-21 15:10 ` [PATCH v2 " Ville Syrjala
2018-09-20 18:51 ` [PATCH 08/18] drm/i915: Use memmove() for punching the hole into infoframes Ville Syrjala
2018-09-21 13:52 ` Daniel Vetter
2018-09-20 18:51 ` [PATCH 09/18] drm/i915: Pass intel_encoder to infoframe functions Ville Syrjala
2018-09-21 13:59 ` Daniel Vetter
2018-09-21 15:03 ` Ville Syrjälä
2018-09-20 18:51 ` [PATCH 10/18] drm/i915: Add the missing HDMI gamut metadata packet stuff Ville Syrjala
2018-09-21 14:15 ` [Intel-gfx] " Daniel Vetter
2018-09-20 18:51 ` [PATCH 11/18] drm/i915: Return the mask of enabled infoframes from ->inforame_enabled() Ville Syrjala
2018-09-24 15:51 ` Daniel Vetter
2018-09-24 16:36 ` [Intel-gfx] " Ville Syrjälä
2018-10-01 6:55 ` Daniel Vetter
2018-10-01 19:29 ` Ville Syrjälä
2018-09-20 18:51 ` [PATCH 12/18] drm/i915: Store mask of enabled infoframes in the crtc state Ville Syrjala
2018-09-24 15:51 ` [Intel-gfx] " Daniel Vetter
2018-09-20 18:51 ` [PATCH 13/18] drm/i915: Precompute HDMI infoframes Ville Syrjala
2018-09-24 15:58 ` Daniel Vetter
2018-09-24 16:42 ` Ville Syrjälä
2018-09-20 18:51 ` [PATCH 14/18] drm/i915: Read out " Ville Syrjala
2018-09-24 16:08 ` Daniel Vetter [this message]
2018-09-24 16:52 ` Ville Syrjälä
2018-09-20 18:51 ` [PATCH 15/18] drm/i915/sdvo: Precompute " Ville Syrjala
2018-09-20 18:51 ` [PATCH 16/18] drm/i915/sdvo: Read out " Ville Syrjala
2018-09-24 16:10 ` [Intel-gfx] " Daniel Vetter
2018-09-24 17:13 ` Ville Syrjälä
2018-10-01 6:59 ` Daniel Vetter
2018-10-01 13:35 ` Ville Syrjälä
2018-10-01 16:48 ` Daniel Vetter
2018-09-20 18:51 ` [PATCH 17/18] drm/i915: Check infoframe state in intel_pipe_config_compare() Ville Syrjala
2018-09-24 16:12 ` Daniel Vetter
2018-10-01 20:35 ` [Intel-gfx] " Ville Syrjälä
2018-10-02 8:16 ` Daniel Vetter
2018-09-20 18:51 ` [PATCH 18/18] drm/i915: Include infoframes in the crtc state dump Ville Syrjala
2018-09-24 16:14 ` [Intel-gfx] " Daniel Vetter
2018-09-20 19:02 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Infoframe precompute/check Patchwork
2018-09-20 19:12 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-09-20 19:26 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-20 22:30 ` ✓ Fi.CI.IGT: " Patchwork
2018-09-21 14:40 ` ✗ Fi.CI.BAT: failure for drm/i915: Infoframe precompute/check (rev2) Patchwork
2018-09-21 15:25 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Infoframe precompute/check (rev4) Patchwork
2018-09-21 15:35 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-09-21 15:46 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-21 17:04 ` ✓ Fi.CI.IGT: " Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180924160809.GA11082@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox