Em Seg, 2016-04-04 às 17:31 +0530, Shashank Sharma escreveu:
> lspcon is a device which acts as DP in some cases
> and as HDMI in some. Here is how:
> - lspcon needs DP(aux) for all the i2c_ove_aux read/write
> transitions so it needs to have some DP level initializations
> - lspcon is detected by userspace/sink as HDMI device, so
> it needs to be detectd as HDMI device.
>
> This patch adds a custom connector for lspcon device, which
> can pick and chose what it wants from existing functionality.
>
> This patch also adds functions to initialize dp and hdmi connectors
> and structures to the minimum required levels, and then play around.
Hi
I realized I only said this in private discussions, but never in the
mailing list, so let's bring it to the list so more people can join:
My problem with this approach of adding a new connector is the amount
of duplicated code it brings. Considering our constant never-ending
code churn, added with the current plans to rework major pieces of the
DP code, I would estimate that this lspcon connector code would very
quickly get out-of-sync and, as a consequence, broken, due to the fact
that it's very likely that someone would forget to patch the lspcon
connector during reworks of DP/HDMI connectors. So my current
suggestion would be to just add the necessary LSPCON bits to the HDMI
and DP detection functions instead of adding a whole new connector.
Maybe we could even merge/integrate the DP and HDMI connector functions
since in the end both functions are probing the same port, and if one
connector is found, the other connector is certainly not present.
I can also provide are more technical lower-level review of the code
itself, but I think we should focus the discussion in the high level
design for now.
Thanks,
Paulo
>
> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_dp.c | 31 +++++++++++++++
> drivers/gpu/drm/i915/intel_drv.h | 4 ++
> drivers/gpu/drm/i915/intel_hdmi.c | 17 ++++++++
> drivers/gpu/drm/i915/intel_lspcon.c | 79
> +++++++++++++++++++++++++++++++++++++
> 4 files changed, 131 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index ba4da0c..f3df1c6 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -5900,6 +5900,37 @@ fail:
> return false;
> }
>
> +int intel_dp_init_minimum(struct intel_digital_port *intel_dig_port,
> + struct intel_connector *intel_connector)
> +{
> + int ret;
> + enum port port = intel_dig_port->port;
> + struct intel_dp *intel_dp = &intel_dig_port->dp;
> + struct drm_device *dev = intel_dig_port->base.base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + intel_dig_port->dp.output_reg = DDI_BUF_CTL(port);
> + if (WARN(intel_dig_port->max_lanes < 1,
> + "Not enough lanes (%d) for DP on port %c\n",
> + intel_dig_port->max_lanes, port_name(port)))
> + return -EINVAL;
> +
> + intel_dp->pps_pipe = INVALID_PIPE;
> + intel_dp->get_aux_clock_divider = skl_get_aux_clock_divider;
> + intel_dp->get_aux_send_ctl = skl_get_aux_send_ctl;
> + intel_dp->prepare_link_retrain =
> intel_ddi_prepare_link_retrain;
> + intel_dp->DP = I915_READ(intel_dp->output_reg);
> + intel_dp->attached_connector = intel_connector;
> + INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
> + edp_panel_vdd_work);
> +
> + ret = intel_dp_aux_init(intel_dp, intel_connector);
> + if (ret)
> + DRM_ERROR("Aux init for LSPCON failed\n");
> +
> + return ret;
> +}
> +
> void
> intel_dp_init(struct drm_device *dev,
> i915_reg_t output_reg, enum port port)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index 6e309ea..d38db7d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1332,6 +1332,8 @@ void intel_dp_compute_rate(struct intel_dp
> *intel_dp, int port_clock,
> bool intel_dp_source_supports_hbr2(struct intel_dp *intel_dp);
> bool
> intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t
> link_status[DP_LINK_STATUS_SIZE]);
> +int intel_dp_init_minimum(struct intel_digital_port *intel_dig_port,
> + struct intel_connector *intel_connector);
>
> /* intel_dp_mst.c */
> int intel_dp_mst_encoder_init(struct intel_digital_port
> *intel_dig_port, int conn_id);
> @@ -1397,6 +1399,8 @@ void intel_fbc_cleanup_cfb(struct
> drm_i915_private *dev_priv);
> void intel_hdmi_init(struct drm_device *dev, i915_reg_t hdmi_reg,
> enum port port);
> void intel_hdmi_init_connector(struct intel_digital_port
> *intel_dig_port,
> struct intel_connector
> *intel_connector);
> +int intel_hdmi_init_minimum(struct intel_digital_port
> *intel_dig_port,
> + struct intel_connector
> *intel_connector);
> struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
> bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> struct intel_crtc_state
> *pipe_config);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> b/drivers/gpu/drm/i915/intel_hdmi.c
> index 22b5a7e..92f5094 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -2128,6 +2128,23 @@ intel_hdmi_add_properties(struct intel_hdmi
> *intel_hdmi, struct drm_connector *c
> intel_hdmi->aspect_ratio = HDMI_PICTURE_ASPECT_NONE;
> }
>
> +int intel_hdmi_init_minimum(struct intel_digital_port
> *intel_dig_port,
> + struct intel_connector
> *intel_connector)
> +{
> + struct intel_hdmi *intel_hdmi = &intel_dig_port->hdmi;
> +
> + if (WARN(intel_dig_port->max_lanes < 4,
> + "Not enough lanes (%d) for HDMI on port %c\n",
> + intel_dig_port->max_lanes, port_name(intel_dig_port-
> >port)))
> + return -EINVAL;
> +
> + intel_hdmi->write_infoframe = hsw_write_infoframe;
> + intel_hdmi->set_infoframes = hsw_set_infoframes;
> + intel_hdmi->infoframe_enabled = hsw_infoframe_enabled;
> + intel_hdmi->attached_connector = intel_connector;
> + return 0;
> +}
> +
> void intel_hdmi_init_connector(struct intel_digital_port
> *intel_dig_port,
> struct intel_connector
> *intel_connector)
> {
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c
> b/drivers/gpu/drm/i915/intel_lspcon.c
> index 5a1993b..e179758 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -54,3 +54,82 @@ struct intel_lspcon *enc_to_lspcon(struct
> drm_encoder *encoder)
> container_of(encoder, struct intel_digital_port,
> base.base);
> return &intel_dig_port->lspcon;
> }
> +
> +int intel_lspcon_init_connector(struct intel_digital_port
> *intel_dig_port)
> +{
> + int ret;
> + struct intel_encoder *intel_encoder = &intel_dig_port->base;
> + struct drm_device *dev = intel_encoder->base.dev;
> + struct drm_i915_private *dev_priv = dev->dev_private;
> + struct intel_connector *intel_connector;
> + struct drm_connector *connector;
> + enum port port = intel_dig_port->port;
> +
> + intel_connector = intel_connector_alloc();
> + if (!intel_connector)
> + return -ENOMEM;
> +
> + connector = &intel_connector->base;
> + connector->interlace_allowed = true;
> + connector->doublescan_allowed = 0;
> +
> + /* init DP */
> + ret = intel_dp_init_minimum(intel_dig_port,
> intel_connector);
> + if (ret) {
> + DRM_ERROR("DP init for LSPCON failed\n");
> + return ret;
> + }
> +
> + /* init HDMI */
> + ret = intel_hdmi_init_minimum(intel_dig_port,
> intel_connector);
> + if (ret) {
> + DRM_ERROR("HDMI init for LSPCON failed\n");
> + return ret;
> + }
> +
> + /* Set up the hotplug pin. */
> + switch (port) {
> + case PORT_A:
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + break;
> + case PORT_B:
> + intel_encoder->hpd_pin = HPD_PORT_B;
> + if (IS_BXT_REVID(dev, 0, BXT_REVID_A1))
> + intel_encoder->hpd_pin = HPD_PORT_A;
> + break;
> + case PORT_C:
> + intel_encoder->hpd_pin = HPD_PORT_C;
> + break;
> + case PORT_D:
> + intel_encoder->hpd_pin = HPD_PORT_D;
> + break;
> + case PORT_E:
> + intel_encoder->hpd_pin = HPD_PORT_E;
> + break;
> + default:
> + DRM_ERROR("Invalid port to configure LSPCON\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * On BXT A0/A1, sw needs to activate DDIA HPD logic and
> + * interrupts to check the external panel connection.
> + */
> + if (IS_BXT_REVID(dev, 0, BXT_REVID_A1) && port == PORT_B)
> + dev_priv->hotplug.irq_port[PORT_A] = intel_dig_port;
> + else
> + dev_priv->hotplug.irq_port[port] = intel_dig_port;
> +
> + /* For G4X desktop chip, PEG_BAND_GAP_DATA 3:0 must first be
> written
> + * 0xd. Failure to do so will result in spurious interrupts
> being
> + * generated on the port when a cable is not attached.
> + */
> + if (IS_G4X(dev) && !IS_GM45(dev)) {
> + u32 temp = I915_READ(PEG_BAND_GAP_DATA);
> +
> + I915_WRITE(PEG_BAND_GAP_DATA, (temp & ~0xf) | 0xd);
> + }
> +
> + DRM_DEBUG_DRIVER("Success: LSPCON connector init\n");
> + return 0;
> +}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
