| Message ID | 20190322015939.23189-2-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] drm/i915/icl: Assign genlock CRTC pointer in all slave CRTC states for tiled displays | expand |
On Thu, 21 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > In case of tiled displays where different tiles are displayed across > different ports, we need to synchronize the transcoders involved. > This patch implements the transcoder port sync feature for > synchronizing one master transcoder with one or more slave > transcoders. This is only enbaled in slave transcoder > and the master transcoder is unaware that it is operating > in this mode. > This has been tested with tiled display connected to ICL. > > Cc: Daniel Vetter <daniel.vetter@intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Matt Roper <matthew.d.roper@intel.com> > Cc: Jani Nikula <jani.nikula@intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++ > 1 file changed, 59 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 9980a4ed8c9c..16b46a3cb3bd 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4009,6 +4009,62 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) > I915_WRITE(PIPE_CHICKEN(pipe), tmp); > } > > +static void icl_set_transcoder_port_sync(struct intel_atomic_state *old_intel_state, > + const struct intel_crtc_state *crtc_state) > +{ > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > + struct intel_crtc_state *genlock_crtc_state = NULL; > + enum transcoder genlock_transcoder; > + u32 trans_ddi_func_ctl2_val; > + u8 master_select; > + > + /* > + * Port Sync Mode cannot be enabled for DP MST > + */ > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > + return; > + > + /* > + * Configure the master select and enable Transcoder Port Sync for > + * Slave CRTCs transcoder. > + */ > + if (!crtc_state->genlock_crtc) > + return; > + > + genlock_crtc_state = intel_atomic_get_new_crtc_state(old_intel_state, > + crtc_state->genlock_crtc); > + if (WARN_ON(!genlock_crtc_state)) > + return; > + > + genlock_transcoder = genlock_crtc_state->cpu_transcoder; > + switch (genlock_transcoder) { > + case TRANSCODER_A: > + master_select = 1; > + break; > + case TRANSCODER_B: > + master_select = 2; > + break; > + case TRANSCODER_C: > + master_select = 3; > + break; > + case TRANSCODER_EDP: > + default: > + master_select = 0; > + break; > + } > + /* Set the master select bits for Tranascoder Port Sync */ > + trans_ddi_func_ctl2_val = I915_READ(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder)); > + trans_ddi_func_ctl2_val |= (PORT_SYNC_MODE_MASTER_SELECT(master_select) & > + PORT_SYNC_MODE_MASTER_SELECT_MASK) << > + PORT_SYNC_MODE_MASTER_SELECT_SHIFT; This doesn't do what you think it does. ITYM, val = I915_READ(); val &= ~FOO_MASK; val |= FOO_BAR; Please actually use just "val" for the variable, the long name just makes this all harder to read. BR, Jani. > + /* Enable Transcoder Port Sync */ > + trans_ddi_func_ctl2_val |= PORT_SYNC_MODE_ENABLE; > + > + I915_WRITE(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder), > + trans_ddi_func_ctl2_val); > +} > + > static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, > const struct intel_crtc_state *new_crtc_state) > { > @@ -5960,6 +6016,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > if (!transcoder_is_dsi(cpu_transcoder)) > intel_set_pipe_timings(pipe_config); > > + if (INTEL_GEN(dev_priv) >= 11) > + icl_set_transcoder_port_sync(old_intel_state, pipe_config); > + > intel_set_pipe_src_size(pipe_config); > > if (cpu_transcoder != TRANSCODER_EDP &&
On Fri, Mar 22, 2019 at 11:34:25AM +0200, Jani Nikula wrote: > On Thu, 21 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > > In case of tiled displays where different tiles are displayed across > > different ports, we need to synchronize the transcoders involved. > > This patch implements the transcoder port sync feature for > > synchronizing one master transcoder with one or more slave > > transcoders. This is only enbaled in slave transcoder > > and the master transcoder is unaware that it is operating > > in this mode. > > This has been tested with tiled display connected to ICL. > > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Matt Roper <matthew.d.roper@intel.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++ > > 1 file changed, 59 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 9980a4ed8c9c..16b46a3cb3bd 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -4009,6 +4009,62 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) > > I915_WRITE(PIPE_CHICKEN(pipe), tmp); > > } > > > > +static void icl_set_transcoder_port_sync(struct intel_atomic_state *old_intel_state, > > + const struct intel_crtc_state *crtc_state) > > +{ > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > + struct intel_crtc_state *genlock_crtc_state = NULL; > > + enum transcoder genlock_transcoder; > > + u32 trans_ddi_func_ctl2_val; > > + u8 master_select; > > + > > + /* > > + * Port Sync Mode cannot be enabled for DP MST > > + */ > > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > > + return; > > + > > + /* > > + * Configure the master select and enable Transcoder Port Sync for > > + * Slave CRTCs transcoder. > > + */ > > + if (!crtc_state->genlock_crtc) > > + return; > > + > > + genlock_crtc_state = intel_atomic_get_new_crtc_state(old_intel_state, > > + crtc_state->genlock_crtc); > > + if (WARN_ON(!genlock_crtc_state)) > > + return; > > + > > + genlock_transcoder = genlock_crtc_state->cpu_transcoder; > > + switch (genlock_transcoder) { > > + case TRANSCODER_A: > > + master_select = 1; > > + break; > > + case TRANSCODER_B: > > + master_select = 2; > > + break; > > + case TRANSCODER_C: > > + master_select = 3; > > + break; > > + case TRANSCODER_EDP: > > + default: > > + master_select = 0; > > + break; > > + } > > + /* Set the master select bits for Tranascoder Port Sync */ > > + trans_ddi_func_ctl2_val = I915_READ(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder)); > > + trans_ddi_func_ctl2_val |= (PORT_SYNC_MODE_MASTER_SELECT(master_select) & > > + PORT_SYNC_MODE_MASTER_SELECT_MASK) << > > + PORT_SYNC_MODE_MASTER_SELECT_SHIFT; > > This doesn't do what you think it does. ITYM, > > val = I915_READ(); > val &= ~FOO_MASK; > val |= FOO_BAR; Also we alreayd have a place where we write this registers. Is there some magic requirement why these bits can't be set there along with eveyrthing else? > > Please actually use just "val" for the variable, the long name just > makes this all harder to read. > > BR, > Jani. > > > > + /* Enable Transcoder Port Sync */ > > + trans_ddi_func_ctl2_val |= PORT_SYNC_MODE_ENABLE; > > + > > + I915_WRITE(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder), > > + trans_ddi_func_ctl2_val); > > +} > > + > > static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, > > const struct intel_crtc_state *new_crtc_state) > > { > > @@ -5960,6 +6016,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > > if (!transcoder_is_dsi(cpu_transcoder)) > > intel_set_pipe_timings(pipe_config); > > > > + if (INTEL_GEN(dev_priv) >= 11) > > + icl_set_transcoder_port_sync(old_intel_state, pipe_config); > > + > > intel_set_pipe_src_size(pipe_config); > > > > if (cpu_transcoder != TRANSCODER_EDP && > > -- > Jani Nikula, Intel Open Source Graphics Center
On Fri, Mar 22, 2019 at 11:34:25AM +0200, Jani Nikula wrote: > On Thu, 21 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > > In case of tiled displays where different tiles are displayed across > > different ports, we need to synchronize the transcoders involved. > > This patch implements the transcoder port sync feature for > > synchronizing one master transcoder with one or more slave > > transcoders. This is only enbaled in slave transcoder > > and the master transcoder is unaware that it is operating > > in this mode. > > This has been tested with tiled display connected to ICL. > > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Cc: Matt Roper <matthew.d.roper@intel.com> > > Cc: Jani Nikula <jani.nikula@intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++ > > 1 file changed, 59 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index 9980a4ed8c9c..16b46a3cb3bd 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -4009,6 +4009,62 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) > > I915_WRITE(PIPE_CHICKEN(pipe), tmp); > > } > > > > +static void icl_set_transcoder_port_sync(struct intel_atomic_state *old_intel_state, > > + const struct intel_crtc_state *crtc_state) > > +{ > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > + struct intel_crtc_state *genlock_crtc_state = NULL; > > + enum transcoder genlock_transcoder; > > + u32 trans_ddi_func_ctl2_val; > > + u8 master_select; > > + > > + /* > > + * Port Sync Mode cannot be enabled for DP MST > > + */ > > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > > + return; > > + > > + /* > > + * Configure the master select and enable Transcoder Port Sync for > > + * Slave CRTCs transcoder. > > + */ > > + if (!crtc_state->genlock_crtc) > > + return; > > + > > + genlock_crtc_state = intel_atomic_get_new_crtc_state(old_intel_state, > > + crtc_state->genlock_crtc); > > + if (WARN_ON(!genlock_crtc_state)) > > + return; > > + > > + genlock_transcoder = genlock_crtc_state->cpu_transcoder; > > + switch (genlock_transcoder) { > > + case TRANSCODER_A: > > + master_select = 1; > > + break; > > + case TRANSCODER_B: > > + master_select = 2; > > + break; > > + case TRANSCODER_C: > > + master_select = 3; > > + break; > > + case TRANSCODER_EDP: > > + default: > > + master_select = 0; > > + break; > > + } > > + /* Set the master select bits for Tranascoder Port Sync */ > > + trans_ddi_func_ctl2_val = I915_READ(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder)); > > + trans_ddi_func_ctl2_val |= (PORT_SYNC_MODE_MASTER_SELECT(master_select) & > > + PORT_SYNC_MODE_MASTER_SELECT_MASK) << > > + PORT_SYNC_MODE_MASTER_SELECT_SHIFT; > > This doesn't do what you think it does. ITYM, > > val = I915_READ(); > val &= ~FOO_MASK; > val |= FOO_BAR; Ah, yes I should use the mask to clear the value and then OR the new value. Will make this change, thanks for pointing this out. > > Please actually use just "val" for the variable, the long name just > makes this all harder to read. Okay will change the variable name to val. Regards Manasi > > BR, > Jani. > > > > + /* Enable Transcoder Port Sync */ > > + trans_ddi_func_ctl2_val |= PORT_SYNC_MODE_ENABLE; > > + > > + I915_WRITE(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder), > > + trans_ddi_func_ctl2_val); > > +} > > + > > static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, > > const struct intel_crtc_state *new_crtc_state) > > { > > @@ -5960,6 +6016,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > > if (!transcoder_is_dsi(cpu_transcoder)) > > intel_set_pipe_timings(pipe_config); > > > > + if (INTEL_GEN(dev_priv) >= 11) > > + icl_set_transcoder_port_sync(old_intel_state, pipe_config); > > + > > intel_set_pipe_src_size(pipe_config); > > > > if (cpu_transcoder != TRANSCODER_EDP && > > -- > Jani Nikula, Intel Open Source Graphics Center
On Fri, Mar 22, 2019 at 03:16:03PM +0200, Ville Syrjälä wrote: > On Fri, Mar 22, 2019 at 11:34:25AM +0200, Jani Nikula wrote: > > On Thu, 21 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > > > In case of tiled displays where different tiles are displayed across > > > different ports, we need to synchronize the transcoders involved. > > > This patch implements the transcoder port sync feature for > > > synchronizing one master transcoder with one or more slave > > > transcoders. This is only enbaled in slave transcoder > > > and the master transcoder is unaware that it is operating > > > in this mode. > > > This has been tested with tiled display connected to ICL. > > > > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++ > > > 1 file changed, 59 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > index 9980a4ed8c9c..16b46a3cb3bd 100644 > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > @@ -4009,6 +4009,62 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) > > > I915_WRITE(PIPE_CHICKEN(pipe), tmp); > > > } > > > > > > +static void icl_set_transcoder_port_sync(struct intel_atomic_state *old_intel_state, > > > + const struct intel_crtc_state *crtc_state) > > > +{ > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > + struct intel_crtc_state *genlock_crtc_state = NULL; > > > + enum transcoder genlock_transcoder; > > > + u32 trans_ddi_func_ctl2_val; > > > + u8 master_select; > > > + > > > + /* > > > + * Port Sync Mode cannot be enabled for DP MST > > > + */ > > > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > > > + return; > > > + > > > + /* > > > + * Configure the master select and enable Transcoder Port Sync for > > > + * Slave CRTCs transcoder. > > > + */ > > > + if (!crtc_state->genlock_crtc) > > > + return; > > > + > > > + genlock_crtc_state = intel_atomic_get_new_crtc_state(old_intel_state, > > > + crtc_state->genlock_crtc); > > > + if (WARN_ON(!genlock_crtc_state)) > > > + return; > > > + > > > + genlock_transcoder = genlock_crtc_state->cpu_transcoder; > > > + switch (genlock_transcoder) { > > > + case TRANSCODER_A: > > > + master_select = 1; > > > + break; > > > + case TRANSCODER_B: > > > + master_select = 2; > > > + break; > > > + case TRANSCODER_C: > > > + master_select = 3; > > > + break; > > > + case TRANSCODER_EDP: > > > + default: > > > + master_select = 0; > > > + break; > > > + } > > > + /* Set the master select bits for Tranascoder Port Sync */ > > > + trans_ddi_func_ctl2_val = I915_READ(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder)); > > > + trans_ddi_func_ctl2_val |= (PORT_SYNC_MODE_MASTER_SELECT(master_select) & > > > + PORT_SYNC_MODE_MASTER_SELECT_MASK) << > > > + PORT_SYNC_MODE_MASTER_SELECT_SHIFT; > > > > This doesn't do what you think it does. ITYM, > > > > val = I915_READ(); > > val &= ~FOO_MASK; > > val |= FOO_BAR; > > Also we alreayd have a place where we write this registers. Is there > some magic requirement why these bits can't be set there along with > eveyrthing else? We only write the bits of TRANS_DDI_FUNC_CTL currently but these bits are written to TRANS_DDI_FUNC_CTL2 and need to be written before enabling the transcoder. Thats why I created this separate function here to set the bits in TRANS_DDI_FUNC_CTL2 Manasi > > > > > Please actually use just "val" for the variable, the long name just > > makes this all harder to read. > > > > BR, > > Jani. > > > > > > > + /* Enable Transcoder Port Sync */ > > > + trans_ddi_func_ctl2_val |= PORT_SYNC_MODE_ENABLE; > > > + > > > + I915_WRITE(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder), > > > + trans_ddi_func_ctl2_val); > > > +} > > > + > > > static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, > > > const struct intel_crtc_state *new_crtc_state) > > > { > > > @@ -5960,6 +6016,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, > > > if (!transcoder_is_dsi(cpu_transcoder)) > > > intel_set_pipe_timings(pipe_config); > > > > > > + if (INTEL_GEN(dev_priv) >= 11) > > > + icl_set_transcoder_port_sync(old_intel_state, pipe_config); > > > + > > > intel_set_pipe_src_size(pipe_config); > > > > > > if (cpu_transcoder != TRANSCODER_EDP && > > > > -- > > Jani Nikula, Intel Open Source Graphics Center > > -- > Ville Syrjälä > Intel
On Fri, Mar 22, 2019 at 10:58:01AM -0700, Manasi Navare wrote: > On Fri, Mar 22, 2019 at 03:16:03PM +0200, Ville Syrjälä wrote: > > On Fri, Mar 22, 2019 at 11:34:25AM +0200, Jani Nikula wrote: > > > On Thu, 21 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > > > > In case of tiled displays where different tiles are displayed across > > > > different ports, we need to synchronize the transcoders involved. > > > > This patch implements the transcoder port sync feature for > > > > synchronizing one master transcoder with one or more slave > > > > transcoders. This is only enbaled in slave transcoder > > > > and the master transcoder is unaware that it is operating > > > > in this mode. > > > > This has been tested with tiled display connected to ICL. > > > > > > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++ > > > > 1 file changed, 59 insertions(+) > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > > index 9980a4ed8c9c..16b46a3cb3bd 100644 > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > @@ -4009,6 +4009,62 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) > > > > I915_WRITE(PIPE_CHICKEN(pipe), tmp); > > > > } > > > > > > > > +static void icl_set_transcoder_port_sync(struct intel_atomic_state *old_intel_state, > > > > + const struct intel_crtc_state *crtc_state) > > > > +{ > > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > > + struct intel_crtc_state *genlock_crtc_state = NULL; > > > > + enum transcoder genlock_transcoder; > > > > + u32 trans_ddi_func_ctl2_val; > > > > + u8 master_select; > > > > + > > > > + /* > > > > + * Port Sync Mode cannot be enabled for DP MST > > > > + */ > > > > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > > > > + return; > > > > + > > > > + /* > > > > + * Configure the master select and enable Transcoder Port Sync for > > > > + * Slave CRTCs transcoder. > > > > + */ > > > > + if (!crtc_state->genlock_crtc) > > > > + return; > > > > + > > > > + genlock_crtc_state = intel_atomic_get_new_crtc_state(old_intel_state, > > > > + crtc_state->genlock_crtc); > > > > + if (WARN_ON(!genlock_crtc_state)) > > > > + return; > > > > + > > > > + genlock_transcoder = genlock_crtc_state->cpu_transcoder; > > > > + switch (genlock_transcoder) { > > > > + case TRANSCODER_A: > > > > + master_select = 1; > > > > + break; > > > > + case TRANSCODER_B: > > > > + master_select = 2; > > > > + break; > > > > + case TRANSCODER_C: > > > > + master_select = 3; > > > > + break; > > > > + case TRANSCODER_EDP: > > > > + default: > > > > + master_select = 0; > > > > + break; > > > > + } > > > > + /* Set the master select bits for Tranascoder Port Sync */ > > > > + trans_ddi_func_ctl2_val = I915_READ(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder)); > > > > + trans_ddi_func_ctl2_val |= (PORT_SYNC_MODE_MASTER_SELECT(master_select) & > > > > + PORT_SYNC_MODE_MASTER_SELECT_MASK) << > > > > + PORT_SYNC_MODE_MASTER_SELECT_SHIFT; > > > > > > This doesn't do what you think it does. ITYM, > > > > > > val = I915_READ(); > > > val &= ~FOO_MASK; > > > val |= FOO_BAR; > > > > Also we alreayd have a place where we write this registers. Is there > > some magic requirement why these bits can't be set there along with > > eveyrthing else? > > We only write the bits of TRANS_DDI_FUNC_CTL currently but these bits > are written to TRANS_DDI_FUNC_CTL2 and need to be written before enabling > the transcoder. > Thats why I created this separate function here to set the bits in TRANS_DDI_FUNC_CTL2 In that case there is no point in doing a rmw.
On Fri, Mar 22, 2019 at 08:09:50PM +0200, Ville Syrjälä wrote: > On Fri, Mar 22, 2019 at 10:58:01AM -0700, Manasi Navare wrote: > > On Fri, Mar 22, 2019 at 03:16:03PM +0200, Ville Syrjälä wrote: > > > On Fri, Mar 22, 2019 at 11:34:25AM +0200, Jani Nikula wrote: > > > > On Thu, 21 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > > > > > In case of tiled displays where different tiles are displayed across > > > > > different ports, we need to synchronize the transcoders involved. > > > > > This patch implements the transcoder port sync feature for > > > > > synchronizing one master transcoder with one or more slave > > > > > transcoders. This is only enbaled in slave transcoder > > > > > and the master transcoder is unaware that it is operating > > > > > in this mode. > > > > > This has been tested with tiled display connected to ICL. > > > > > > > > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++ > > > > > 1 file changed, 59 insertions(+) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > > > index 9980a4ed8c9c..16b46a3cb3bd 100644 > > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > > @@ -4009,6 +4009,62 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) > > > > > I915_WRITE(PIPE_CHICKEN(pipe), tmp); > > > > > } > > > > > > > > > > +static void icl_set_transcoder_port_sync(struct intel_atomic_state *old_intel_state, > > > > > + const struct intel_crtc_state *crtc_state) > > > > > +{ > > > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > > > + struct intel_crtc_state *genlock_crtc_state = NULL; > > > > > + enum transcoder genlock_transcoder; > > > > > + u32 trans_ddi_func_ctl2_val; > > > > > + u8 master_select; > > > > > + > > > > > + /* > > > > > + * Port Sync Mode cannot be enabled for DP MST > > > > > + */ > > > > > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > > > > > + return; > > > > > + > > > > > + /* > > > > > + * Configure the master select and enable Transcoder Port Sync for > > > > > + * Slave CRTCs transcoder. > > > > > + */ > > > > > + if (!crtc_state->genlock_crtc) > > > > > + return; > > > > > + > > > > > + genlock_crtc_state = intel_atomic_get_new_crtc_state(old_intel_state, > > > > > + crtc_state->genlock_crtc); > > > > > + if (WARN_ON(!genlock_crtc_state)) > > > > > + return; > > > > > + > > > > > + genlock_transcoder = genlock_crtc_state->cpu_transcoder; > > > > > + switch (genlock_transcoder) { > > > > > + case TRANSCODER_A: > > > > > + master_select = 1; > > > > > + break; > > > > > + case TRANSCODER_B: > > > > > + master_select = 2; > > > > > + break; > > > > > + case TRANSCODER_C: > > > > > + master_select = 3; > > > > > + break; > > > > > + case TRANSCODER_EDP: > > > > > + default: > > > > > + master_select = 0; > > > > > + break; > > > > > + } > > > > > + /* Set the master select bits for Tranascoder Port Sync */ > > > > > + trans_ddi_func_ctl2_val = I915_READ(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder)); > > > > > + trans_ddi_func_ctl2_val |= (PORT_SYNC_MODE_MASTER_SELECT(master_select) & > > > > > + PORT_SYNC_MODE_MASTER_SELECT_MASK) << > > > > > + PORT_SYNC_MODE_MASTER_SELECT_SHIFT; > > > > > > > > This doesn't do what you think it does. ITYM, > > > > > > > > val = I915_READ(); > > > > val &= ~FOO_MASK; > > > > val |= FOO_BAR; > > > > > > Also we alreayd have a place where we write this registers. Is there > > > some magic requirement why these bits can't be set there along with > > > eveyrthing else? > > > > We only write the bits of TRANS_DDI_FUNC_CTL currently but these bits > > are written to TRANS_DDI_FUNC_CTL2 and need to be written before enabling > > the transcoder. > > Thats why I created this separate function here to set the bits in TRANS_DDI_FUNC_CTL2 > > In that case there is no point in doing a rmw. But isnt it always a good idea to do rmw? I mean what if the master select was set to something else earlier? Also could you look at the patch 1 of this series that assigns the genlock crtc pointer? Manasi > > -- > Ville Syrjälä > Intel
On Fri, Mar 22, 2019 at 11:44:21AM -0700, Manasi Navare wrote: > On Fri, Mar 22, 2019 at 08:09:50PM +0200, Ville Syrjälä wrote: > > On Fri, Mar 22, 2019 at 10:58:01AM -0700, Manasi Navare wrote: > > > On Fri, Mar 22, 2019 at 03:16:03PM +0200, Ville Syrjälä wrote: > > > > On Fri, Mar 22, 2019 at 11:34:25AM +0200, Jani Nikula wrote: > > > > > On Thu, 21 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > > > > > > In case of tiled displays where different tiles are displayed across > > > > > > different ports, we need to synchronize the transcoders involved. > > > > > > This patch implements the transcoder port sync feature for > > > > > > synchronizing one master transcoder with one or more slave > > > > > > transcoders. This is only enbaled in slave transcoder > > > > > > and the master transcoder is unaware that it is operating > > > > > > in this mode. > > > > > > This has been tested with tiled display connected to ICL. > > > > > > > > > > > > Cc: Daniel Vetter <daniel.vetter@intel.com> > > > > > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > Cc: Matt Roper <matthew.d.roper@intel.com> > > > > > > Cc: Jani Nikula <jani.nikula@intel.com> > > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > > --- > > > > > > drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++ > > > > > > 1 file changed, 59 insertions(+) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > > > > > index 9980a4ed8c9c..16b46a3cb3bd 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_display.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c > > > > > > @@ -4009,6 +4009,62 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) > > > > > > I915_WRITE(PIPE_CHICKEN(pipe), tmp); > > > > > > } > > > > > > > > > > > > +static void icl_set_transcoder_port_sync(struct intel_atomic_state *old_intel_state, > > > > > > + const struct intel_crtc_state *crtc_state) > > > > > > +{ > > > > > > + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > > > > > > + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > > > > > > + struct intel_crtc_state *genlock_crtc_state = NULL; > > > > > > + enum transcoder genlock_transcoder; > > > > > > + u32 trans_ddi_func_ctl2_val; > > > > > > + u8 master_select; > > > > > > + > > > > > > + /* > > > > > > + * Port Sync Mode cannot be enabled for DP MST > > > > > > + */ > > > > > > + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) > > > > > > + return; > > > > > > + > > > > > > + /* > > > > > > + * Configure the master select and enable Transcoder Port Sync for > > > > > > + * Slave CRTCs transcoder. > > > > > > + */ > > > > > > + if (!crtc_state->genlock_crtc) > > > > > > + return; > > > > > > + > > > > > > + genlock_crtc_state = intel_atomic_get_new_crtc_state(old_intel_state, > > > > > > + crtc_state->genlock_crtc); > > > > > > + if (WARN_ON(!genlock_crtc_state)) > > > > > > + return; > > > > > > + > > > > > > + genlock_transcoder = genlock_crtc_state->cpu_transcoder; > > > > > > + switch (genlock_transcoder) { > > > > > > + case TRANSCODER_A: > > > > > > + master_select = 1; > > > > > > + break; > > > > > > + case TRANSCODER_B: > > > > > > + master_select = 2; > > > > > > + break; > > > > > > + case TRANSCODER_C: > > > > > > + master_select = 3; > > > > > > + break; > > > > > > + case TRANSCODER_EDP: > > > > > > + default: > > > > > > + master_select = 0; > > > > > > + break; > > > > > > + } > > > > > > + /* Set the master select bits for Tranascoder Port Sync */ > > > > > > + trans_ddi_func_ctl2_val = I915_READ(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder)); > > > > > > + trans_ddi_func_ctl2_val |= (PORT_SYNC_MODE_MASTER_SELECT(master_select) & > > > > > > + PORT_SYNC_MODE_MASTER_SELECT_MASK) << > > > > > > + PORT_SYNC_MODE_MASTER_SELECT_SHIFT; > > > > > > > > > > This doesn't do what you think it does. ITYM, > > > > > > > > > > val = I915_READ(); > > > > > val &= ~FOO_MASK; > > > > > val |= FOO_BAR; > > > > > > > > Also we alreayd have a place where we write this registers. Is there > > > > some magic requirement why these bits can't be set there along with > > > > eveyrthing else? > > > > > > We only write the bits of TRANS_DDI_FUNC_CTL currently but these bits > > > are written to TRANS_DDI_FUNC_CTL2 and need to be written before enabling > > > the transcoder. > > > Thats why I created this separate function here to set the bits in TRANS_DDI_FUNC_CTL2 > > > > In that case there is no point in doing a rmw. > > But isnt it always a good idea to do rmw? I mean what if the master select was set to something else > earlier? RMW is the root of many evils. It should be avoided unless there is a really compelling reason to use it.
On Fri, 22 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > On Fri, Mar 22, 2019 at 11:44:21AM -0700, Manasi Navare wrote: >> On Fri, Mar 22, 2019 at 08:09:50PM +0200, Ville Syrjälä wrote: >> > In that case there is no point in doing a rmw. >> >> But isnt it always a good idea to do rmw? I mean what if the master >> select was set to something else earlier? > > RMW is the root of many evils. It should be avoided unless there is a > really compelling reason to use it. Hear, hear! We have the software state that we want to write to the hardware. If we use RMW to do this, it might all work by coincidence due to the old values in the registers, or it might just as well break by coincidence due to some garbage in the registers. In most cases, there should only be one place that writes a particular display register during modeset. Sometimes this isn't possible, and RMW is required. Some registers also have reserved bits potentially used by the hardware that must not be changed, and RMW is required. These are documented in bspec. BR, Jani.
On Fri, Mar 22, 2019 at 09:28:01PM +0200, Jani Nikula wrote: > On Fri, 22 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > > On Fri, Mar 22, 2019 at 11:44:21AM -0700, Manasi Navare wrote: > >> On Fri, Mar 22, 2019 at 08:09:50PM +0200, Ville Syrjälä wrote: > >> > In that case there is no point in doing a rmw. > >> > >> But isnt it always a good idea to do rmw? I mean what if the master > >> select was set to something else earlier? > > > > RMW is the root of many evils. It should be avoided unless there is a > > really compelling reason to use it. > > Hear, hear! > > We have the software state that we want to write to the hardware. If we > use RMW to do this, it might all work by coincidence due to the old > values in the registers, or it might just as well break by coincidence > due to some garbage in the registers. > > In most cases, there should only be one place that writes a particular > display register during modeset. Sometimes this isn't possible, and RMW > is required. > > Some registers also have reserved bits potentially used by the hardware > that must not be changed, and RMW is required. These are documented in > bspec. > > BR, > Jani. > Thanks for the explanation. It does make sense now that we are doing a full modeset, we should just be then writing the value directly? The only concern I have is that say DSI code sets this somewhere els ein the modeset path, then we would need to modify this to do RMW or always make sure DSI also uses the same function for writing to this reg. What do you suggest doing now? Manasi > > -- > Jani Nikula, Intel Open Source Graphics Center
On Fri, 22 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > On Fri, Mar 22, 2019 at 09:28:01PM +0200, Jani Nikula wrote: >> On Fri, 22 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> > On Fri, Mar 22, 2019 at 11:44:21AM -0700, Manasi Navare wrote: >> >> On Fri, Mar 22, 2019 at 08:09:50PM +0200, Ville Syrjälä wrote: >> >> > In that case there is no point in doing a rmw. >> >> >> >> But isnt it always a good idea to do rmw? I mean what if the master >> >> select was set to something else earlier? >> > >> > RMW is the root of many evils. It should be avoided unless there is a >> > really compelling reason to use it. >> >> Hear, hear! >> >> We have the software state that we want to write to the hardware. If we >> use RMW to do this, it might all work by coincidence due to the old >> values in the registers, or it might just as well break by coincidence >> due to some garbage in the registers. >> >> In most cases, there should only be one place that writes a particular >> display register during modeset. Sometimes this isn't possible, and RMW >> is required. >> >> Some registers also have reserved bits potentially used by the hardware >> that must not be changed, and RMW is required. These are documented in >> bspec. >> >> BR, >> Jani. >> > > Thanks for the explanation. It does make sense now that we are doing a > full modeset, we should just be then writing the value directly? The > only concern I have is that say DSI code sets this somewhere els ein > the modeset path, then we would need to modify this to do RMW or > always make sure DSI also uses the same function for writing to this > reg. What do you suggest doing now? I think all encoders in a tile group are always of the same type. If the tile grouping in your patch is based purely on EDID, we may need to enforce this. Surely genlock only works on encoders of the same type? In any case DSI (at least currently) does not use tile groups, and will never be mixed up in non-DSI tile groups. The DSI transcoders are separate from other transcoders, so we're not writing the same registers here. --- Looking at the code, I am wondering if this should be pushed to encoder hooks instead of adding into crtc enable. BR, Jani.
On Thu, Mar 28, 2019 at 11:18:56AM +0200, Jani Nikula wrote: > On Fri, 22 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > > On Fri, Mar 22, 2019 at 09:28:01PM +0200, Jani Nikula wrote: > >> On Fri, 22 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > >> > On Fri, Mar 22, 2019 at 11:44:21AM -0700, Manasi Navare wrote: > >> >> On Fri, Mar 22, 2019 at 08:09:50PM +0200, Ville Syrjälä wrote: > >> >> > In that case there is no point in doing a rmw. > >> >> > >> >> But isnt it always a good idea to do rmw? I mean what if the master > >> >> select was set to something else earlier? > >> > > >> > RMW is the root of many evils. It should be avoided unless there is a > >> > really compelling reason to use it. > >> > >> Hear, hear! > >> > >> We have the software state that we want to write to the hardware. If we > >> use RMW to do this, it might all work by coincidence due to the old > >> values in the registers, or it might just as well break by coincidence > >> due to some garbage in the registers. > >> > >> In most cases, there should only be one place that writes a particular > >> display register during modeset. Sometimes this isn't possible, and RMW > >> is required. > >> > >> Some registers also have reserved bits potentially used by the hardware > >> that must not be changed, and RMW is required. These are documented in > >> bspec. > >> > >> BR, > >> Jani. > >> > > > > Thanks for the explanation. It does make sense now that we are doing a > > full modeset, we should just be then writing the value directly? The > > only concern I have is that say DSI code sets this somewhere els ein > > the modeset path, then we would need to modify this to do RMW or > > always make sure DSI also uses the same function for writing to this > > reg. What do you suggest doing now? > > I think all encoders in a tile group are always of the same type. Yes all the encoders in tile group are always same type. > > If the tile grouping in your patch is based purely on EDID, we may need > to enforce this. Surely genlock only works on encoders of the same type? > So all the slaves and their master will always be of same type and yes it is based on the EDID tile block parsing. But just to double sure I think when i assign the master slave pointers, I should check that the connector type is the same. > In any case DSI (at least currently) does not use tile groups, and will > never be mixed up in non-DSI tile groups. The DSI transcoders are > separate from other transcoders, so we're not writing the same registers > here. > > --- > > Looking at the code, I am wondering if this should be pushed to encoder > hooks instead of adding into crtc enable. As per the Bspec sequence, this needs to happen before enabling the TRANS_DDI_FUNC_CTL and after the link training, so I put in the crtc_enable hook, which encoder hooks are you suggesting adding this? Regards Manasi > > BR, > Jani. > > > > -- > Jani Nikula, Intel Open Source Graphics Center
On Thu, 28 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: > On Thu, Mar 28, 2019 at 11:18:56AM +0200, Jani Nikula wrote: >> On Fri, 22 Mar 2019, Manasi Navare <manasi.d.navare@intel.com> wrote: >> > On Fri, Mar 22, 2019 at 09:28:01PM +0200, Jani Nikula wrote: >> >> On Fri, 22 Mar 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >> >> > On Fri, Mar 22, 2019 at 11:44:21AM -0700, Manasi Navare wrote: >> >> >> On Fri, Mar 22, 2019 at 08:09:50PM +0200, Ville Syrjälä wrote: >> >> >> > In that case there is no point in doing a rmw. >> >> >> >> >> >> But isnt it always a good idea to do rmw? I mean what if the master >> >> >> select was set to something else earlier? >> >> > >> >> > RMW is the root of many evils. It should be avoided unless there is a >> >> > really compelling reason to use it. >> >> >> >> Hear, hear! >> >> >> >> We have the software state that we want to write to the hardware. If we >> >> use RMW to do this, it might all work by coincidence due to the old >> >> values in the registers, or it might just as well break by coincidence >> >> due to some garbage in the registers. >> >> >> >> In most cases, there should only be one place that writes a particular >> >> display register during modeset. Sometimes this isn't possible, and RMW >> >> is required. >> >> >> >> Some registers also have reserved bits potentially used by the hardware >> >> that must not be changed, and RMW is required. These are documented in >> >> bspec. >> >> >> >> BR, >> >> Jani. >> >> >> > >> > Thanks for the explanation. It does make sense now that we are doing a >> > full modeset, we should just be then writing the value directly? The >> > only concern I have is that say DSI code sets this somewhere els ein >> > the modeset path, then we would need to modify this to do RMW or >> > always make sure DSI also uses the same function for writing to this >> > reg. What do you suggest doing now? >> >> I think all encoders in a tile group are always of the same type. > > Yes all the encoders in tile group are always same type. > >> >> If the tile grouping in your patch is based purely on EDID, we may need >> to enforce this. Surely genlock only works on encoders of the same type? >> > > So all the slaves and their master will always be of same type and yes it is > based on the EDID tile block parsing. > But just to double sure I think when i assign the master slave pointers, I should > check that the connector type is the same. > >> In any case DSI (at least currently) does not use tile groups, and will >> never be mixed up in non-DSI tile groups. The DSI transcoders are >> separate from other transcoders, so we're not writing the same registers >> here. >> >> --- >> >> Looking at the code, I am wondering if this should be pushed to encoder >> hooks instead of adding into crtc enable. > > As per the Bspec sequence, this needs to happen before enabling the > TRANS_DDI_FUNC_CTL and after the link training, so I put in the > crtc_enable hook, which encoder hooks are you suggesting adding this? Maybe go with what you have now first, this can be pushed to encoders later if needed. (I hope I don't regret this. ;) BR, Jani. > > Regards > Manasi >> >> BR, >> Jani. >> >> >> >> -- >> Jani Nikula, Intel Open Source Graphics Center
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 9980a4ed8c9c..16b46a3cb3bd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4009,6 +4009,62 @@ static void icl_set_pipe_chicken(struct intel_crtc *crtc) I915_WRITE(PIPE_CHICKEN(pipe), tmp); } +static void icl_set_transcoder_port_sync(struct intel_atomic_state *old_intel_state, + const struct intel_crtc_state *crtc_state) +{ + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + struct intel_crtc_state *genlock_crtc_state = NULL; + enum transcoder genlock_transcoder; + u32 trans_ddi_func_ctl2_val; + u8 master_select; + + /* + * Port Sync Mode cannot be enabled for DP MST + */ + if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST)) + return; + + /* + * Configure the master select and enable Transcoder Port Sync for + * Slave CRTCs transcoder. + */ + if (!crtc_state->genlock_crtc) + return; + + genlock_crtc_state = intel_atomic_get_new_crtc_state(old_intel_state, + crtc_state->genlock_crtc); + if (WARN_ON(!genlock_crtc_state)) + return; + + genlock_transcoder = genlock_crtc_state->cpu_transcoder; + switch (genlock_transcoder) { + case TRANSCODER_A: + master_select = 1; + break; + case TRANSCODER_B: + master_select = 2; + break; + case TRANSCODER_C: + master_select = 3; + break; + case TRANSCODER_EDP: + default: + master_select = 0; + break; + } + /* Set the master select bits for Tranascoder Port Sync */ + trans_ddi_func_ctl2_val = I915_READ(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder)); + trans_ddi_func_ctl2_val |= (PORT_SYNC_MODE_MASTER_SELECT(master_select) & + PORT_SYNC_MODE_MASTER_SELECT_MASK) << + PORT_SYNC_MODE_MASTER_SELECT_SHIFT; + /* Enable Transcoder Port Sync */ + trans_ddi_func_ctl2_val |= PORT_SYNC_MODE_ENABLE; + + I915_WRITE(TRANS_DDI_FUNC_CTL2(crtc_state->cpu_transcoder), + trans_ddi_func_ctl2_val); +} + static void intel_update_pipe_config(const struct intel_crtc_state *old_crtc_state, const struct intel_crtc_state *new_crtc_state) { @@ -5960,6 +6016,9 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, if (!transcoder_is_dsi(cpu_transcoder)) intel_set_pipe_timings(pipe_config); + if (INTEL_GEN(dev_priv) >= 11) + icl_set_transcoder_port_sync(old_intel_state, pipe_config); + intel_set_pipe_src_size(pipe_config); if (cpu_transcoder != TRANSCODER_EDP &&
In case of tiled displays where different tiles are displayed across different ports, we need to synchronize the transcoders involved. This patch implements the transcoder port sync feature for synchronizing one master transcoder with one or more slave transcoders. This is only enbaled in slave transcoder and the master transcoder is unaware that it is operating in this mode. This has been tested with tiled display connected to ICL. Cc: Daniel Vetter <daniel.vetter@intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Matt Roper <matthew.d.roper@intel.com> Cc: Jani Nikula <jani.nikula@intel.com> Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> --- drivers/gpu/drm/i915/intel_display.c | 59 ++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+)