| Message ID | 20190828072823.4832-6-ramalingam.c@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Enable HDCP 1.4 and 2.2 on Gen12+ | expand |
> On gen12+ platforms, HDCP HW is associated to the transcoder. > Hence on every modeset update associated transcoder into the intel_hdcp of > the port. > > v2: > s/trans/cpu_transcoder [Jani] > v3: > comment is added for fw_ddi init for gen12+ [Shashank] > only hdcp capable transcoder is translated into fw_tc [Shashank] > v4: > fw_tc initialization is kept for modeset. [Tomas] > few extra doc is added at port_data init [Tomas] > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > Acked-by: Jani Nikula <jani.nikula@intel.com> > --- > .../drm/i915/display/intel_display_types.h | 7 +++ > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ > drivers/gpu/drm/i915/display/intel_hdcp.c | 48 ++++++++++++++++++- > drivers/gpu/drm/i915/display/intel_hdcp.h | 3 ++ > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++ > 5 files changed, 63 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > b/drivers/gpu/drm/i915/display/intel_display_types.h > index 96514dcc7812..61277a87dbe7 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > @@ -388,6 +388,13 @@ struct intel_hdcp { > wait_queue_head_t cp_irq_queue; > atomic_t cp_irq_count; > int cp_irq_count_cached; > + > + /* > + * HDCP register access for gen12+ need the transcoder associated. > + * Transcoder attached to the connector could be changed at modeset. > + * Hence caching the transcoder here. > + */ > + enum transcoder cpu_transcoder; > }; > > struct intel_connector { > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > b/drivers/gpu/drm/i915/display/intel_dp.c > index 23908da1cd5d..e8471689f785 100644 > --- a/drivers/gpu/drm/i915/display/intel_dp.c > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > @@ -2248,6 +2248,9 @@ intel_dp_compute_config(struct intel_encoder > *encoder, > > intel_psr_compute_config(intel_dp, pipe_config); > > + intel_hdcp_transcoder_config(intel_connector, > + pipe_config->cpu_transcoder); > + > return 0; > } > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > b/drivers/gpu/drm/i915/display/intel_hdcp.c > index e8b04cc8fcb1..988d675f9f69 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > @@ -1764,13 +1764,59 @@ enum mei_fw_ddi > intel_get_mei_fw_ddi_index(enum port port) > } > } > > +static inline > +enum mei_fw_tc intel_get_mei_fw_tc(enum transcoder cpu_transcoder) { > + switch (cpu_transcoder) { > + case TRANSCODER_A ... TRANSCODER_D: > + return (enum mei_fw_tc)(cpu_transcoder | 0x10); > + default: /* eDP, DSI TRANSCODERS are non HDCP capable */ > + return MEI_INVALID_TRANSCODER; > + } > +} > + > +void intel_hdcp_transcoder_config(struct intel_connector *connector, > + enum transcoder cpu_transcoder) > +{ > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > + struct intel_hdcp *hdcp = &connector->hdcp; > + > + if (!hdcp->shim) > + return; > + > + if (INTEL_GEN(dev_priv) >= 12) { > + mutex_lock(&hdcp->mutex); > + hdcp->cpu_transcoder = cpu_transcoder; > + hdcp->port_data.fw_tc = > intel_get_mei_fw_tc(cpu_transcoder); > + mutex_unlock(&hdcp->mutex); > + } > +} > + > static inline int initialize_hdcp_port_data(struct intel_connector *connector, > const struct intel_hdcp_shim *shim) > { > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > struct intel_hdcp *hdcp = &connector->hdcp; > struct hdcp_port_data *data = &hdcp->port_data; > > - data->fw_ddi = intel_get_mei_fw_ddi_index(connector->encoder- > >port); > + if (INTEL_GEN(dev_priv) < 12) { Now you have one liners, you should drop the brackets. > + data->fw_ddi = > + intel_get_mei_fw_ddi_index(connector->encoder- > >port); > + } else { > + > + /* > + * As per ME FW API expectation, for GEN 12+, fw_ddi is filled > + * with INVALID PORT index. > + */ > + data->fw_ddi = MEI_DDI_INVALID_PORT; In your previous patch you commented that this should be '0', need to make comment consistent even if MEI_DDI_INVALID_PORT is 0 > + } > + > + /* > + * As associated transcoder is set and modified at modeset, here fw_tc > + * is initialized to invalid index. > + */ > + data->fw_tc = MEI_INVALID_TRANSCODER; In your previous you commented this should be '0', as for port. > + > data->port_type = (u8)HDCP_PORT_TYPE_INTEGRATED; > data->protocol = (u8)shim->protocol; > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h > b/drivers/gpu/drm/i915/display/intel_hdcp.h > index 59a2b40405cc..41c1053d9e38 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h > @@ -16,10 +16,13 @@ struct drm_i915_private; struct intel_connector; struct > intel_hdcp_shim; enum port; > +enum transcoder; > > void intel_hdcp_atomic_check(struct drm_connector *connector, > struct drm_connector_state *old_state, > struct drm_connector_state *new_state); > +void intel_hdcp_transcoder_config(struct intel_connector *connector, > + enum transcoder cpu_transcoder); > int intel_hdcp_init(struct intel_connector *connector, > const struct intel_hdcp_shim *hdcp_shim); int > intel_hdcp_enable(struct intel_connector *connector, u8 content_type); diff -- > git a/drivers/gpu/drm/i915/display/intel_hdmi.c > b/drivers/gpu/drm/i915/display/intel_hdmi.c > index e02f0faecf02..6e9bb6bd1ee2 100644 > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > @@ -2431,6 +2431,9 @@ int intel_hdmi_compute_config(struct intel_encoder > *encoder, > return -EINVAL; > } > > + intel_hdcp_transcoder_config(intel_hdmi->attached_connector, > + pipe_config->cpu_transcoder); > + > return 0; > } > > -- > 2.20.1
On 2019-08-28 at 13:56:06 +0530, Winkler, Tomas wrote: > > > On gen12+ platforms, HDCP HW is associated to the transcoder. > > Hence on every modeset update associated transcoder into the intel_hdcp of > > the port. > > > > v2: > > s/trans/cpu_transcoder [Jani] > > v3: > > comment is added for fw_ddi init for gen12+ [Shashank] > > only hdcp capable transcoder is translated into fw_tc [Shashank] > > v4: > > fw_tc initialization is kept for modeset. [Tomas] > > few extra doc is added at port_data init [Tomas] > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > > Acked-by: Jani Nikula <jani.nikula@intel.com> > > --- > > .../drm/i915/display/intel_display_types.h | 7 +++ > > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ > > drivers/gpu/drm/i915/display/intel_hdcp.c | 48 ++++++++++++++++++- > > drivers/gpu/drm/i915/display/intel_hdcp.h | 3 ++ > > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++ > > 5 files changed, 63 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > index 96514dcc7812..61277a87dbe7 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > @@ -388,6 +388,13 @@ struct intel_hdcp { > > wait_queue_head_t cp_irq_queue; > > atomic_t cp_irq_count; > > int cp_irq_count_cached; > > + > > + /* > > + * HDCP register access for gen12+ need the transcoder associated. > > + * Transcoder attached to the connector could be changed at modeset. > > + * Hence caching the transcoder here. > > + */ > > + enum transcoder cpu_transcoder; > > }; > > > > struct intel_connector { > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > b/drivers/gpu/drm/i915/display/intel_dp.c > > index 23908da1cd5d..e8471689f785 100644 > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > @@ -2248,6 +2248,9 @@ intel_dp_compute_config(struct intel_encoder > > *encoder, > > > > intel_psr_compute_config(intel_dp, pipe_config); > > > > + intel_hdcp_transcoder_config(intel_connector, > > + pipe_config->cpu_transcoder); > > + > > return 0; > > } > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > index e8b04cc8fcb1..988d675f9f69 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > @@ -1764,13 +1764,59 @@ enum mei_fw_ddi > > intel_get_mei_fw_ddi_index(enum port port) > > } > > } > > > > +static inline > > +enum mei_fw_tc intel_get_mei_fw_tc(enum transcoder cpu_transcoder) { > > + switch (cpu_transcoder) { > > + case TRANSCODER_A ... TRANSCODER_D: > > + return (enum mei_fw_tc)(cpu_transcoder | 0x10); > > + default: /* eDP, DSI TRANSCODERS are non HDCP capable */ > > + return MEI_INVALID_TRANSCODER; > > + } > > +} > > + > > +void intel_hdcp_transcoder_config(struct intel_connector *connector, > > + enum transcoder cpu_transcoder) > > +{ > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > + struct intel_hdcp *hdcp = &connector->hdcp; > > + > > + if (!hdcp->shim) > > + return; > > + > > + if (INTEL_GEN(dev_priv) >= 12) { > > + mutex_lock(&hdcp->mutex); > > + hdcp->cpu_transcoder = cpu_transcoder; > > + hdcp->port_data.fw_tc = > > intel_get_mei_fw_tc(cpu_transcoder); > > + mutex_unlock(&hdcp->mutex); > > + } > > +} > > + > > static inline int initialize_hdcp_port_data(struct intel_connector *connector, > > const struct intel_hdcp_shim *shim) > > { > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > struct intel_hdcp *hdcp = &connector->hdcp; > > struct hdcp_port_data *data = &hdcp->port_data; > > > > - data->fw_ddi = intel_get_mei_fw_ddi_index(connector->encoder- > > >port); > > + if (INTEL_GEN(dev_priv) < 12) { > Now you have one liners, you should drop the brackets. > > + data->fw_ddi = > > + intel_get_mei_fw_ddi_index(connector->encoder- > > >port); > > + } else { > > + > > + /* > > + * As per ME FW API expectation, for GEN 12+, fw_ddi is filled > > + * with INVALID PORT index. > > + */ > > + data->fw_ddi = MEI_DDI_INVALID_PORT; > In your previous patch you commented that this should be '0', need to make comment consistent even if MEI_DDI_INVALID_PORT is 0 > > + } > > + > > + /* > > + * As associated transcoder is set and modified at modeset, here fw_tc > > + * is initialized to invalid index. > > + */ > > + data->fw_tc = MEI_INVALID_TRANSCODER; > In your previous you commented this should be '0', as for port. True! assigning invalid transcoder and port index as they are zero and meeting the expectation from ME FW API expectation. May be I will explain that in comment too!? Sounds good to you? -Ram > > > + > > data->port_type = (u8)HDCP_PORT_TYPE_INTEGRATED; > > data->protocol = (u8)shim->protocol; > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h > > b/drivers/gpu/drm/i915/display/intel_hdcp.h > > index 59a2b40405cc..41c1053d9e38 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.h > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h > > @@ -16,10 +16,13 @@ struct drm_i915_private; struct intel_connector; struct > > intel_hdcp_shim; enum port; > > +enum transcoder; > > > > void intel_hdcp_atomic_check(struct drm_connector *connector, > > struct drm_connector_state *old_state, > > struct drm_connector_state *new_state); > > +void intel_hdcp_transcoder_config(struct intel_connector *connector, > > + enum transcoder cpu_transcoder); > > int intel_hdcp_init(struct intel_connector *connector, > > const struct intel_hdcp_shim *hdcp_shim); int > > intel_hdcp_enable(struct intel_connector *connector, u8 content_type); diff -- > > git a/drivers/gpu/drm/i915/display/intel_hdmi.c > > b/drivers/gpu/drm/i915/display/intel_hdmi.c > > index e02f0faecf02..6e9bb6bd1ee2 100644 > > --- a/drivers/gpu/drm/i915/display/intel_hdmi.c > > +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c > > @@ -2431,6 +2431,9 @@ int intel_hdmi_compute_config(struct intel_encoder > > *encoder, > > return -EINVAL; > > } > > > > + intel_hdcp_transcoder_config(intel_hdmi->attached_connector, > > + pipe_config->cpu_transcoder); > > + > > return 0; > > } > > > > -- > > 2.20.1 >
> > On 2019-08-28 at 13:56:06 +0530, Winkler, Tomas wrote: > > > > > On gen12+ platforms, HDCP HW is associated to the transcoder. > > > Hence on every modeset update associated transcoder into the > > > intel_hdcp of the port. > > > > > > v2: > > > s/trans/cpu_transcoder [Jani] > > > v3: > > > comment is added for fw_ddi init for gen12+ [Shashank] > > > only hdcp capable transcoder is translated into fw_tc [Shashank] > > > v4: > > > fw_tc initialization is kept for modeset. [Tomas] > > > few extra doc is added at port_data init [Tomas] > > > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > > > Acked-by: Jani Nikula <jani.nikula@intel.com> > > > --- > > > .../drm/i915/display/intel_display_types.h | 7 +++ > > > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 48 ++++++++++++++++++- > > > drivers/gpu/drm/i915/display/intel_hdcp.h | 3 ++ > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++ > > > 5 files changed, 63 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > > index 96514dcc7812..61277a87dbe7 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > @@ -388,6 +388,13 @@ struct intel_hdcp { > > > wait_queue_head_t cp_irq_queue; > > > atomic_t cp_irq_count; > > > int cp_irq_count_cached; > > > + > > > + /* > > > + * HDCP register access for gen12+ need the transcoder associated. > > > + * Transcoder attached to the connector could be changed at modeset. > > > + * Hence caching the transcoder here. > > > + */ > > > + enum transcoder cpu_transcoder; > > > }; > > > > > > struct intel_connector { > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > index 23908da1cd5d..e8471689f785 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > @@ -2248,6 +2248,9 @@ intel_dp_compute_config(struct intel_encoder > > > *encoder, > > > > > > intel_psr_compute_config(intel_dp, pipe_config); > > > > > > + intel_hdcp_transcoder_config(intel_connector, > > > + pipe_config->cpu_transcoder); > > > + > > > return 0; > > > } > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > index e8b04cc8fcb1..988d675f9f69 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > @@ -1764,13 +1764,59 @@ enum mei_fw_ddi > > > intel_get_mei_fw_ddi_index(enum port port) > > > } > > > } > > > > > > +static inline > > > +enum mei_fw_tc intel_get_mei_fw_tc(enum transcoder cpu_transcoder) { > > > + switch (cpu_transcoder) { > > > + case TRANSCODER_A ... TRANSCODER_D: > > > + return (enum mei_fw_tc)(cpu_transcoder | 0x10); > > > + default: /* eDP, DSI TRANSCODERS are non HDCP capable */ > > > + return MEI_INVALID_TRANSCODER; > > > + } > > > +} > > > + > > > +void intel_hdcp_transcoder_config(struct intel_connector *connector, > > > + enum transcoder cpu_transcoder) { > > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > > + struct intel_hdcp *hdcp = &connector->hdcp; > > > + > > > + if (!hdcp->shim) > > > + return; > > > + > > > + if (INTEL_GEN(dev_priv) >= 12) { > > > + mutex_lock(&hdcp->mutex); > > > + hdcp->cpu_transcoder = cpu_transcoder; > > > + hdcp->port_data.fw_tc = > > > intel_get_mei_fw_tc(cpu_transcoder); > > > + mutex_unlock(&hdcp->mutex); > > > + } > > > +} > > > + > > > static inline int initialize_hdcp_port_data(struct intel_connector > *connector, > > > const struct intel_hdcp_shim *shim) > { > > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > > struct intel_hdcp *hdcp = &connector->hdcp; > > > struct hdcp_port_data *data = &hdcp->port_data; > > > > > > - data->fw_ddi = intel_get_mei_fw_ddi_index(connector->encoder- > > > >port); > > > + if (INTEL_GEN(dev_priv) < 12) { > > Now you have one liners, you should drop the brackets. > > > + data->fw_ddi = > > > + intel_get_mei_fw_ddi_index(connector->encoder- > > > >port); > > > + } else { > > > + > > > + /* > > > + * As per ME FW API expectation, for GEN 12+, fw_ddi is filled > > > + * with INVALID PORT index. > > > + */ > > > + data->fw_ddi = MEI_DDI_INVALID_PORT; > > In your previous patch you commented that this should be '0', need to > > make comment consistent even if MEI_DDI_INVALID_PORT is 0 > > > + } > > > + > > > + /* > > > + * As associated transcoder is set and modified at modeset, here fw_tc > > > + * is initialized to invalid index. > > > + */ > > > + data->fw_tc = MEI_INVALID_TRANSCODER; > > In your previous you commented this should be '0', as for port. > True! assigning invalid transcoder and port index as they are zero and meeting > the expectation from ME FW API expectation. May be I will explain that in > comment too!? Sounds good to you? Sure, whatever makes it clear.
On 2019-08-28 at 14:09:01 +0530, Winkler, Tomas wrote: > > > > On 2019-08-28 at 13:56:06 +0530, Winkler, Tomas wrote: > > > > > > > On gen12+ platforms, HDCP HW is associated to the transcoder. > > > > Hence on every modeset update associated transcoder into the > > > > intel_hdcp of the port. > > > > > > > > v2: > > > > s/trans/cpu_transcoder [Jani] > > > > v3: > > > > comment is added for fw_ddi init for gen12+ [Shashank] > > > > only hdcp capable transcoder is translated into fw_tc [Shashank] > > > > v4: > > > > fw_tc initialization is kept for modeset. [Tomas] > > > > few extra doc is added at port_data init [Tomas] > > > > > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > > > > Acked-by: Jani Nikula <jani.nikula@intel.com> > > > > --- > > > > .../drm/i915/display/intel_display_types.h | 7 +++ > > > > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ > > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 48 ++++++++++++++++++- > > > > drivers/gpu/drm/i915/display/intel_hdcp.h | 3 ++ > > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++ > > > > 5 files changed, 63 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > index 96514dcc7812..61277a87dbe7 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > @@ -388,6 +388,13 @@ struct intel_hdcp { > > > > wait_queue_head_t cp_irq_queue; > > > > atomic_t cp_irq_count; > > > > int cp_irq_count_cached; > > > > + > > > > + /* > > > > + * HDCP register access for gen12+ need the transcoder associated. > > > > + * Transcoder attached to the connector could be changed at modeset. > > > > + * Hence caching the transcoder here. > > > > + */ > > > > + enum transcoder cpu_transcoder; > > > > }; > > > > > > > > struct intel_connector { > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > > index 23908da1cd5d..e8471689f785 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > @@ -2248,6 +2248,9 @@ intel_dp_compute_config(struct intel_encoder > > > > *encoder, > > > > > > > > intel_psr_compute_config(intel_dp, pipe_config); > > > > > > > > + intel_hdcp_transcoder_config(intel_connector, > > > > + pipe_config->cpu_transcoder); > > > > + > > > > return 0; > > > > } > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > index e8b04cc8fcb1..988d675f9f69 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > @@ -1764,13 +1764,59 @@ enum mei_fw_ddi > > > > intel_get_mei_fw_ddi_index(enum port port) > > > > } > > > > } > > > > > > > > +static inline > > > > +enum mei_fw_tc intel_get_mei_fw_tc(enum transcoder cpu_transcoder) { > > > > + switch (cpu_transcoder) { > > > > + case TRANSCODER_A ... TRANSCODER_D: > > > > + return (enum mei_fw_tc)(cpu_transcoder | 0x10); > > > > + default: /* eDP, DSI TRANSCODERS are non HDCP capable */ > > > > + return MEI_INVALID_TRANSCODER; > > > > + } > > > > +} > > > > + > > > > +void intel_hdcp_transcoder_config(struct intel_connector *connector, > > > > + enum transcoder cpu_transcoder) { > > > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > > > + struct intel_hdcp *hdcp = &connector->hdcp; > > > > + > > > > + if (!hdcp->shim) > > > > + return; > > > > + > > > > + if (INTEL_GEN(dev_priv) >= 12) { > > > > + mutex_lock(&hdcp->mutex); > > > > + hdcp->cpu_transcoder = cpu_transcoder; > > > > + hdcp->port_data.fw_tc = > > > > intel_get_mei_fw_tc(cpu_transcoder); > > > > + mutex_unlock(&hdcp->mutex); > > > > + } > > > > +} > > > > + > > > > static inline int initialize_hdcp_port_data(struct intel_connector > > *connector, > > > > const struct intel_hdcp_shim *shim) > > { > > > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > > > struct intel_hdcp *hdcp = &connector->hdcp; > > > > struct hdcp_port_data *data = &hdcp->port_data; > > > > > > > > - data->fw_ddi = intel_get_mei_fw_ddi_index(connector->encoder- > > > > >port); > > > > + if (INTEL_GEN(dev_priv) < 12) { > > > Now you have one liners, you should drop the brackets. > > > > + data->fw_ddi = > > > > + intel_get_mei_fw_ddi_index(connector->encoder- > > > > >port); > > > > + } else { > > > > + > > > > + /* > > > > + * As per ME FW API expectation, for GEN 12+, fw_ddi is filled > > > > + * with INVALID PORT index. > > > > + */ > > > > + data->fw_ddi = MEI_DDI_INVALID_PORT; > > > In your previous patch you commented that this should be '0', need to > > > make comment consistent even if MEI_DDI_INVALID_PORT is 0 > > > > + } > > > > + > > > > + /* > > > > + * As associated transcoder is set and modified at modeset, here fw_tc > > > > + * is initialized to invalid index. > > > > + */ > > > > + data->fw_tc = MEI_INVALID_TRANSCODER; > > > In your previous you commented this should be '0', as for port. > > True! assigning invalid transcoder and port index as they are zero and meeting > > the expectation from ME FW API expectation. May be I will explain that in > > comment too!? Sounds good to you? > Sure, whatever makes it clear. Like below: /* * As per ME FW API expectation, for GEN 12+, fw_ddi is filled * with zero(INVALID PORT index) */ data->fw_ddi = MEI_DDI_INVALID_PORT; } /* * As associated transcoder is set and modified at modeset, here fw_tc * is initialized to zero (invalid transcoder index) This will * be retained for <Gen12. */ data->fw_tc = MEI_INVALID_TRANSCODER; -Ram > >
> > On 2019-08-28 at 14:09:01 +0530, Winkler, Tomas wrote: > > > > > > On 2019-08-28 at 13:56:06 +0530, Winkler, Tomas wrote: > > > > > > > > > On gen12+ platforms, HDCP HW is associated to the transcoder. > > > > > Hence on every modeset update associated transcoder into the > > > > > intel_hdcp of the port. > > > > > > > > > > v2: > > > > > s/trans/cpu_transcoder [Jani] > > > > > v3: > > > > > comment is added for fw_ddi init for gen12+ [Shashank] > > > > > only hdcp capable transcoder is translated into fw_tc > > > > > [Shashank] > > > > > v4: > > > > > fw_tc initialization is kept for modeset. [Tomas] > > > > > few extra doc is added at port_data init [Tomas] > > > > > > > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > > > > > Acked-by: Jani Nikula <jani.nikula@intel.com> > > > > > --- > > > > > .../drm/i915/display/intel_display_types.h | 7 +++ > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ > > > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 48 > ++++++++++++++++++- > > > > > drivers/gpu/drm/i915/display/intel_hdcp.h | 3 ++ > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++ > > > > > 5 files changed, 63 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > index 96514dcc7812..61277a87dbe7 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > @@ -388,6 +388,13 @@ struct intel_hdcp { > > > > > wait_queue_head_t cp_irq_queue; > > > > > atomic_t cp_irq_count; > > > > > int cp_irq_count_cached; > > > > > + > > > > > + /* > > > > > + * HDCP register access for gen12+ need the transcoder > associated. > > > > > + * Transcoder attached to the connector could be changed at > modeset. > > > > > + * Hence caching the transcoder here. > > > > > + */ > > > > > + enum transcoder cpu_transcoder; > > > > > }; > > > > > > > > > > struct intel_connector { > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > index 23908da1cd5d..e8471689f785 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > @@ -2248,6 +2248,9 @@ intel_dp_compute_config(struct > > > > > intel_encoder *encoder, > > > > > > > > > > intel_psr_compute_config(intel_dp, pipe_config); > > > > > > > > > > + intel_hdcp_transcoder_config(intel_connector, > > > > > + pipe_config->cpu_transcoder); > > > > > + > > > > > return 0; > > > > > } > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > > index e8b04cc8fcb1..988d675f9f69 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > > @@ -1764,13 +1764,59 @@ enum mei_fw_ddi > > > > > intel_get_mei_fw_ddi_index(enum port port) > > > > > } > > > > > } > > > > > > > > > > +static inline > > > > > +enum mei_fw_tc intel_get_mei_fw_tc(enum transcoder > cpu_transcoder) { > > > > > + switch (cpu_transcoder) { > > > > > + case TRANSCODER_A ... TRANSCODER_D: > > > > > + return (enum mei_fw_tc)(cpu_transcoder | 0x10); > > > > > + default: /* eDP, DSI TRANSCODERS are non HDCP capable */ > > > > > + return MEI_INVALID_TRANSCODER; > > > > > + } > > > > > +} > > > > > + > > > > > +void intel_hdcp_transcoder_config(struct intel_connector *connector, > > > > > + enum transcoder cpu_transcoder) { > > > > > + struct drm_i915_private *dev_priv = to_i915(connector- > >base.dev); > > > > > + struct intel_hdcp *hdcp = &connector->hdcp; > > > > > + > > > > > + if (!hdcp->shim) > > > > > + return; > > > > > + > > > > > + if (INTEL_GEN(dev_priv) >= 12) { > > > > > + mutex_lock(&hdcp->mutex); > > > > > + hdcp->cpu_transcoder = cpu_transcoder; > > > > > + hdcp->port_data.fw_tc = > > > > > intel_get_mei_fw_tc(cpu_transcoder); > > > > > + mutex_unlock(&hdcp->mutex); > > > > > + } > > > > > +} > > > > > + > > > > > static inline int initialize_hdcp_port_data(struct > > > > > intel_connector > > > *connector, > > > > > const struct > intel_hdcp_shim *shim) > > > { > > > > > + struct drm_i915_private *dev_priv = > > > > > +to_i915(connector->base.dev); > > > > > struct intel_hdcp *hdcp = &connector->hdcp; > > > > > struct hdcp_port_data *data = &hdcp->port_data; > > > > > > > > > > - data->fw_ddi = intel_get_mei_fw_ddi_index(connector- > >encoder- > > > > > >port); > > > > > + if (INTEL_GEN(dev_priv) < 12) { > > > > Now you have one liners, you should drop the brackets. > > > > > + data->fw_ddi = > > > > > + intel_get_mei_fw_ddi_index(connector- > >encoder- > > > > > >port); > > > > > + } else { > > > > > + > > > > > + /* > > > > > + * As per ME FW API expectation, for GEN 12+, fw_ddi > is filled > > > > > + * with INVALID PORT index. > > > > > + */ > > > > > + data->fw_ddi = MEI_DDI_INVALID_PORT; > > > > In your previous patch you commented that this should be '0', need > > > > to make comment consistent even if MEI_DDI_INVALID_PORT is 0 > > > > > + } > > > > > + > > > > > + /* > > > > > + * As associated transcoder is set and modified at modeset, > here fw_tc > > > > > + * is initialized to invalid index. > > > > > + */ > > > > > + data->fw_tc = MEI_INVALID_TRANSCODER; > > > > In your previous you commented this should be '0', as for port. > > > True! assigning invalid transcoder and port index as they are zero > > > and meeting the expectation from ME FW API expectation. May be I > > > will explain that in comment too!? Sounds good to you? > > Sure, whatever makes it clear. > Like below: OK. > > /* > * As per ME FW API expectation, for GEN 12+, fw_ddi is filled > * with zero(INVALID PORT index) > */ > data->fw_ddi = MEI_DDI_INVALID_PORT; > } > > /* > * As associated transcoder is set and modified at modeset, here fw_tc > * is initialized to zero (invalid transcoder index) This will > * be retained for <Gen12. > */ > data->fw_tc = MEI_INVALID_TRANSCODER; > > -Ram > > > >
Looks good to me, From display side, Please feel free to use Reviewed-by: Shashank Sharma <shashank.sharma@intel.com> > -----Original Message----- > From: C, Ramalingam > Sent: Wednesday, August 28, 2019 2:28 PM > To: Winkler, Tomas <tomas.winkler@intel.com> > Cc: intel-gfx <intel-gfx@lists.freedesktop.org>; dri-devel <dri- > devel@lists.freedesktop.org>; Sharma, Shashank <shashank.sharma@intel.com>; > Daniel Vetter <daniel@ffwll.ch>; Nikula, Jani <jani.nikula@intel.com> > Subject: Re: [PATCH v11 5/6] drm/i915/hdcp: update current transcoder into > intel_hdcp > > On 2019-08-28 at 14:09:01 +0530, Winkler, Tomas wrote: > > > > > > On 2019-08-28 at 13:56:06 +0530, Winkler, Tomas wrote: > > > > > > > > > On gen12+ platforms, HDCP HW is associated to the transcoder. > > > > > Hence on every modeset update associated transcoder into the > > > > > intel_hdcp of the port. > > > > > > > > > > v2: > > > > > s/trans/cpu_transcoder [Jani] > > > > > v3: > > > > > comment is added for fw_ddi init for gen12+ [Shashank] > > > > > only hdcp capable transcoder is translated into fw_tc > > > > > [Shashank] > > > > > v4: > > > > > fw_tc initialization is kept for modeset. [Tomas] > > > > > few extra doc is added at port_data init [Tomas] > > > > > > > > > > Signed-off-by: Ramalingam C <ramalingam.c@intel.com> > > > > > Acked-by: Jani Nikula <jani.nikula@intel.com> > > > > > --- > > > > > .../drm/i915/display/intel_display_types.h | 7 +++ > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 3 ++ > > > > > drivers/gpu/drm/i915/display/intel_hdcp.c | 48 ++++++++++++++++++- > > > > > drivers/gpu/drm/i915/display/intel_hdcp.h | 3 ++ > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 3 ++ > > > > > 5 files changed, 63 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > index 96514dcc7812..61277a87dbe7 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h > > > > > @@ -388,6 +388,13 @@ struct intel_hdcp { > > > > > wait_queue_head_t cp_irq_queue; > > > > > atomic_t cp_irq_count; > > > > > int cp_irq_count_cached; > > > > > + > > > > > + /* > > > > > + * HDCP register access for gen12+ need the transcoder associated. > > > > > + * Transcoder attached to the connector could be changed at modeset. > > > > > + * Hence caching the transcoder here. > > > > > + */ > > > > > + enum transcoder cpu_transcoder; > > > > > }; > > > > > > > > > > struct intel_connector { > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > index 23908da1cd5d..e8471689f785 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c > > > > > @@ -2248,6 +2248,9 @@ intel_dp_compute_config(struct > > > > > intel_encoder *encoder, > > > > > > > > > > intel_psr_compute_config(intel_dp, pipe_config); > > > > > > > > > > + intel_hdcp_transcoder_config(intel_connector, > > > > > + pipe_config->cpu_transcoder); > > > > > + > > > > > return 0; > > > > > } > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > > b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > > index e8b04cc8fcb1..988d675f9f69 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c > > > > > @@ -1764,13 +1764,59 @@ enum mei_fw_ddi > > > > > intel_get_mei_fw_ddi_index(enum port port) > > > > > } > > > > > } > > > > > > > > > > +static inline > > > > > +enum mei_fw_tc intel_get_mei_fw_tc(enum transcoder cpu_transcoder) { > > > > > + switch (cpu_transcoder) { > > > > > + case TRANSCODER_A ... TRANSCODER_D: > > > > > + return (enum mei_fw_tc)(cpu_transcoder | 0x10); > > > > > + default: /* eDP, DSI TRANSCODERS are non HDCP capable */ > > > > > + return MEI_INVALID_TRANSCODER; > > > > > + } > > > > > +} > > > > > + > > > > > +void intel_hdcp_transcoder_config(struct intel_connector *connector, > > > > > + enum transcoder cpu_transcoder) { > > > > > + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); > > > > > + struct intel_hdcp *hdcp = &connector->hdcp; > > > > > + > > > > > + if (!hdcp->shim) > > > > > + return; > > > > > + > > > > > + if (INTEL_GEN(dev_priv) >= 12) { > > > > > + mutex_lock(&hdcp->mutex); > > > > > + hdcp->cpu_transcoder = cpu_transcoder; > > > > > + hdcp->port_data.fw_tc = > > > > > intel_get_mei_fw_tc(cpu_transcoder); > > > > > + mutex_unlock(&hdcp->mutex); > > > > > + } > > > > > +} > > > > > + > > > > > static inline int initialize_hdcp_port_data(struct > > > > > intel_connector > > > *connector, > > > > > const struct intel_hdcp_shim *shim) > > > { > > > > > + struct drm_i915_private *dev_priv = > > > > > +to_i915(connector->base.dev); > > > > > struct intel_hdcp *hdcp = &connector->hdcp; > > > > > struct hdcp_port_data *data = &hdcp->port_data; > > > > > > > > > > - data->fw_ddi = intel_get_mei_fw_ddi_index(connector->encoder- > > > > > >port); > > > > > + if (INTEL_GEN(dev_priv) < 12) { > > > > Now you have one liners, you should drop the brackets. > > > > > + data->fw_ddi = > > > > > + intel_get_mei_fw_ddi_index(connector->encoder- > > > > > >port); > > > > > + } else { > > > > > + > > > > > + /* > > > > > + * As per ME FW API expectation, for GEN 12+, fw_ddi is filled > > > > > + * with INVALID PORT index. > > > > > + */ > > > > > + data->fw_ddi = MEI_DDI_INVALID_PORT; > > > > In your previous patch you commented that this should be '0', need > > > > to make comment consistent even if MEI_DDI_INVALID_PORT is 0 > > > > > + } > > > > > + > > > > > + /* > > > > > + * As associated transcoder is set and modified at modeset, here fw_tc > > > > > + * is initialized to invalid index. > > > > > + */ > > > > > + data->fw_tc = MEI_INVALID_TRANSCODER; > > > > In your previous you commented this should be '0', as for port. > > > True! assigning invalid transcoder and port index as they are zero > > > and meeting the expectation from ME FW API expectation. May be I > > > will explain that in comment too!? Sounds good to you? > > Sure, whatever makes it clear. > Like below: > > /* > * As per ME FW API expectation, for GEN 12+, fw_ddi is filled > * with zero(INVALID PORT index) > */ > data->fw_ddi = MEI_DDI_INVALID_PORT; > } > > /* > * As associated transcoder is set and modified at modeset, here fw_tc > * is initialized to zero (invalid transcoder index) This will > * be retained for <Gen12. > */ > data->fw_tc = MEI_INVALID_TRANSCODER; > > -Ram > > > >
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h index 96514dcc7812..61277a87dbe7 100644 --- a/drivers/gpu/drm/i915/display/intel_display_types.h +++ b/drivers/gpu/drm/i915/display/intel_display_types.h @@ -388,6 +388,13 @@ struct intel_hdcp { wait_queue_head_t cp_irq_queue; atomic_t cp_irq_count; int cp_irq_count_cached; + + /* + * HDCP register access for gen12+ need the transcoder associated. + * Transcoder attached to the connector could be changed at modeset. + * Hence caching the transcoder here. + */ + enum transcoder cpu_transcoder; }; struct intel_connector { diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c index 23908da1cd5d..e8471689f785 100644 --- a/drivers/gpu/drm/i915/display/intel_dp.c +++ b/drivers/gpu/drm/i915/display/intel_dp.c @@ -2248,6 +2248,9 @@ intel_dp_compute_config(struct intel_encoder *encoder, intel_psr_compute_config(intel_dp, pipe_config); + intel_hdcp_transcoder_config(intel_connector, + pipe_config->cpu_transcoder); + return 0; } diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.c b/drivers/gpu/drm/i915/display/intel_hdcp.c index e8b04cc8fcb1..988d675f9f69 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.c +++ b/drivers/gpu/drm/i915/display/intel_hdcp.c @@ -1764,13 +1764,59 @@ enum mei_fw_ddi intel_get_mei_fw_ddi_index(enum port port) } } +static inline +enum mei_fw_tc intel_get_mei_fw_tc(enum transcoder cpu_transcoder) +{ + switch (cpu_transcoder) { + case TRANSCODER_A ... TRANSCODER_D: + return (enum mei_fw_tc)(cpu_transcoder | 0x10); + default: /* eDP, DSI TRANSCODERS are non HDCP capable */ + return MEI_INVALID_TRANSCODER; + } +} + +void intel_hdcp_transcoder_config(struct intel_connector *connector, + enum transcoder cpu_transcoder) +{ + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); + struct intel_hdcp *hdcp = &connector->hdcp; + + if (!hdcp->shim) + return; + + if (INTEL_GEN(dev_priv) >= 12) { + mutex_lock(&hdcp->mutex); + hdcp->cpu_transcoder = cpu_transcoder; + hdcp->port_data.fw_tc = intel_get_mei_fw_tc(cpu_transcoder); + mutex_unlock(&hdcp->mutex); + } +} + static inline int initialize_hdcp_port_data(struct intel_connector *connector, const struct intel_hdcp_shim *shim) { + struct drm_i915_private *dev_priv = to_i915(connector->base.dev); struct intel_hdcp *hdcp = &connector->hdcp; struct hdcp_port_data *data = &hdcp->port_data; - data->fw_ddi = intel_get_mei_fw_ddi_index(connector->encoder->port); + if (INTEL_GEN(dev_priv) < 12) { + data->fw_ddi = + intel_get_mei_fw_ddi_index(connector->encoder->port); + } else { + + /* + * As per ME FW API expectation, for GEN 12+, fw_ddi is filled + * with INVALID PORT index. + */ + data->fw_ddi = MEI_DDI_INVALID_PORT; + } + + /* + * As associated transcoder is set and modified at modeset, here fw_tc + * is initialized to invalid index. + */ + data->fw_tc = MEI_INVALID_TRANSCODER; + data->port_type = (u8)HDCP_PORT_TYPE_INTEGRATED; data->protocol = (u8)shim->protocol; diff --git a/drivers/gpu/drm/i915/display/intel_hdcp.h b/drivers/gpu/drm/i915/display/intel_hdcp.h index 59a2b40405cc..41c1053d9e38 100644 --- a/drivers/gpu/drm/i915/display/intel_hdcp.h +++ b/drivers/gpu/drm/i915/display/intel_hdcp.h @@ -16,10 +16,13 @@ struct drm_i915_private; struct intel_connector; struct intel_hdcp_shim; enum port; +enum transcoder; void intel_hdcp_atomic_check(struct drm_connector *connector, struct drm_connector_state *old_state, struct drm_connector_state *new_state); +void intel_hdcp_transcoder_config(struct intel_connector *connector, + enum transcoder cpu_transcoder); int intel_hdcp_init(struct intel_connector *connector, const struct intel_hdcp_shim *hdcp_shim); int intel_hdcp_enable(struct intel_connector *connector, u8 content_type); diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c index e02f0faecf02..6e9bb6bd1ee2 100644 --- a/drivers/gpu/drm/i915/display/intel_hdmi.c +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c @@ -2431,6 +2431,9 @@ int intel_hdmi_compute_config(struct intel_encoder *encoder, return -EINVAL; } + intel_hdcp_transcoder_config(intel_hdmi->attached_connector, + pipe_config->cpu_transcoder); + return 0; }