Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-21 Thread Manasi Navare
As per the feedback, I tested the code with saving master_transcoder instead of 
master_crtc
and in the HW state readout I loop through all transcoders to fill the 
trans_port_sync_slaves bitmask.
This is working great and i dont see any pipe state mismatches now.

Thanks for all the feedback and I will be sending the new patch revision out 
later today.

Regards
Manasi

On Thu, Jun 13, 2019 at 01:13:28PM -0700, Manasi Navare wrote:
> On Thu, Jun 13, 2019 at 08:12:37PM +0300, Ville Syrjälä wrote:
> > On Thu, Jun 13, 2019 at 09:41:42AM -0700, Manasi Navare wrote:
> > > On Thu, Jun 13, 2019 at 12:10:57PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Jun 12, 2019 at 01:59:52PM -0700, Manasi Navare wrote:
> > > > > On Wed, Jun 12, 2019 at 10:30:55PM +0300, Ville Syrjälä wrote:
> > > > > > On Wed, Jun 12, 2019 at 12:11:02PM -0700, Manasi Navare wrote:
> > > > > > > On Wed, Jun 12, 2019 at 10:04:26PM +0300, Ville Syrjälä wrote:
> > > > > > > > On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst 
> > > > > > > > wrote:
> > > > > > > > > Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > > > > > > > > > In case of tiled displays when the two tiles are sent 
> > > > > > > > > > across two CRTCs
> > > > > > > > > > over two separate DP SST connectors, we need a mechanism to 
> > > > > > > > > > synchronize
> > > > > > > > > > the two CRTCs and their corresponding transcoders.
> > > > > > > > > > So use the master-slave mode where there is one master 
> > > > > > > > > > corresponding
> > > > > > > > > > to last horizontal and vertical tile that needs to be 
> > > > > > > > > > genlocked with
> > > > > > > > > > all other slave tiles.
> > > > > > > > > > This patch identifies saves the master CRTC pointer in all 
> > > > > > > > > > the slave
> > > > > > > > > > CRTC states. This pointer is needed to select the master 
> > > > > > > > > > CRTC/transcoder
> > > > > > > > > > while configuring transcoder port sync for the 
> > > > > > > > > > corresponding slaves.
> > > > > > > > > >
> > > > > > > > > > v2:
> > > > > > > > > > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > > > > > > > > > * Use slave_bitmask to save associated slaves in master 
> > > > > > > > > > crtc state (Ville)
> > > > > > > > > >
> > > > > > > > > > Cc: Daniel Vetter 
> > > > > > > > > > Cc: Ville Syrjälä 
> > > > > > > > > > Cc: Maarten Lankhorst 
> > > > > > > > > > Cc: Matt Roper 
> > > > > > > > > > Signed-off-by: Manasi Navare 
> > > > > > > > > > ---
> > > > > > > > > >  drivers/gpu/drm/i915/intel_display.c | 89 
> > > > > > > > > > 
> > > > > > > > > >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > > > > > > > > >  2 files changed, 95 insertions(+)
> > > > > > > > > >
> > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > > > > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > > > index b276345779e6..92dea2231499 100644
> > > > > > > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > > > @@ -11316,6 +11316,86 @@ static int 
> > > > > > > > > > icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
> > > > > > > > > > return 0;
> > > > > > > > > >  }
> > > > > > > > > >  
> > > > > > > > > > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > > > > > > > > > +struct intel_crtc_state 
> > > > > > > > > > *crtc_state,
> > > > > > > > > > +struct drm_atomic_state *state)
> > > > > > > > > > +{
> > > > > > > > > > +   struct drm_i915_private *dev_priv = 
> > > > > > > > > > to_i915(crtc_state->base.crtc->dev);
> > > > > > > > > > +   struct drm_connector *master_connector, *connector;
> > > > > > > > > > +   struct drm_connector_state *connector_state;
> > > > > > > > > > +   struct drm_connector_list_iter conn_iter;
> > > > > > > > > > +   struct drm_crtc *master_crtc = NULL;
> > > > > > > > > > +   struct drm_crtc_state *master_crtc_state;
> > > > > > > > > > +   int i, tile_group_id;
> > > > > > > > > > +
> > > > > > > > > > +   if (INTEL_GEN(dev_priv) < 11)
> > > > > > > > > > +   return 0;
> > > > > > > > > > +
> > > > > > > > > > +   /*
> > > > > > > > > > +* In case of tiled displays there could be one or more 
> > > > > > > > > > slaves but there is
> > > > > > > > > > +* only one master. Lets make the CRTC used by the 
> > > > > > > > > > connector corresponding
> > > > > > > > > > +* to the last horizonal and last vertical tile a 
> > > > > > > > > > master/genlock CRTC.
> > > > > > > > > > +* All the other CRTCs corresponding to other tiles of 
> > > > > > > > > > the same Tile group
> > > > > > > > > > +* are the slave CRTCs and hold a pointer to their 
> > > > > > > > > > genlock CRTC.
> > > > > > > > > > +*/
> > > > > > > > > > +   for_each_new_connector_in_state(state, connector, 
> > > > > > > > > > connector_state, i) {
> > > > > > > > > > +   if 

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-13 Thread Manasi Navare
On Thu, Jun 13, 2019 at 08:12:37PM +0300, Ville Syrjälä wrote:
> On Thu, Jun 13, 2019 at 09:41:42AM -0700, Manasi Navare wrote:
> > On Thu, Jun 13, 2019 at 12:10:57PM +0300, Ville Syrjälä wrote:
> > > On Wed, Jun 12, 2019 at 01:59:52PM -0700, Manasi Navare wrote:
> > > > On Wed, Jun 12, 2019 at 10:30:55PM +0300, Ville Syrjälä wrote:
> > > > > On Wed, Jun 12, 2019 at 12:11:02PM -0700, Manasi Navare wrote:
> > > > > > On Wed, Jun 12, 2019 at 10:04:26PM +0300, Ville Syrjälä wrote:
> > > > > > > On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst wrote:
> > > > > > > > Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > > > > > > > > In case of tiled displays when the two tiles are sent across 
> > > > > > > > > two CRTCs
> > > > > > > > > over two separate DP SST connectors, we need a mechanism to 
> > > > > > > > > synchronize
> > > > > > > > > the two CRTCs and their corresponding transcoders.
> > > > > > > > > So use the master-slave mode where there is one master 
> > > > > > > > > corresponding
> > > > > > > > > to last horizontal and vertical tile that needs to be 
> > > > > > > > > genlocked with
> > > > > > > > > all other slave tiles.
> > > > > > > > > This patch identifies saves the master CRTC pointer in all 
> > > > > > > > > the slave
> > > > > > > > > CRTC states. This pointer is needed to select the master 
> > > > > > > > > CRTC/transcoder
> > > > > > > > > while configuring transcoder port sync for the corresponding 
> > > > > > > > > slaves.
> > > > > > > > >
> > > > > > > > > v2:
> > > > > > > > > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > > > > > > > > * Use slave_bitmask to save associated slaves in master crtc 
> > > > > > > > > state (Ville)
> > > > > > > > >
> > > > > > > > > Cc: Daniel Vetter 
> > > > > > > > > Cc: Ville Syrjälä 
> > > > > > > > > Cc: Maarten Lankhorst 
> > > > > > > > > Cc: Matt Roper 
> > > > > > > > > Signed-off-by: Manasi Navare 
> > > > > > > > > ---
> > > > > > > > >  drivers/gpu/drm/i915/intel_display.c | 89 
> > > > > > > > > 
> > > > > > > > >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > > > > > > > >  2 files changed, 95 insertions(+)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > > > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > > index b276345779e6..92dea2231499 100644
> > > > > > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > > @@ -11316,6 +11316,86 @@ static int 
> > > > > > > > > icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
> > > > > > > > >   return 0;
> > > > > > > > >  }
> > > > > > > > >  
> > > > > > > > > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > > > > > > > > +  struct intel_crtc_state 
> > > > > > > > > *crtc_state,
> > > > > > > > > +  struct drm_atomic_state *state)
> > > > > > > > > +{
> > > > > > > > > + struct drm_i915_private *dev_priv = 
> > > > > > > > > to_i915(crtc_state->base.crtc->dev);
> > > > > > > > > + struct drm_connector *master_connector, *connector;
> > > > > > > > > + struct drm_connector_state *connector_state;
> > > > > > > > > + struct drm_connector_list_iter conn_iter;
> > > > > > > > > + struct drm_crtc *master_crtc = NULL;
> > > > > > > > > + struct drm_crtc_state *master_crtc_state;
> > > > > > > > > + int i, tile_group_id;
> > > > > > > > > +
> > > > > > > > > + if (INTEL_GEN(dev_priv) < 11)
> > > > > > > > > + return 0;
> > > > > > > > > +
> > > > > > > > > + /*
> > > > > > > > > +  * In case of tiled displays there could be one or more 
> > > > > > > > > slaves but there is
> > > > > > > > > +  * only one master. Lets make the CRTC used by the 
> > > > > > > > > connector corresponding
> > > > > > > > > +  * to the last horizonal and last vertical tile a 
> > > > > > > > > master/genlock CRTC.
> > > > > > > > > +  * All the other CRTCs corresponding to other tiles of 
> > > > > > > > > the same Tile group
> > > > > > > > > +  * are the slave CRTCs and hold a pointer to their 
> > > > > > > > > genlock CRTC.
> > > > > > > > > +  */
> > > > > > > > > + for_each_new_connector_in_state(state, connector, 
> > > > > > > > > connector_state, i) {
> > > > > > > > > + if (connector_state->crtc != crtc)
> > > > > > > > > + continue;
> > > > > > > > > + if (!connector->has_tile)
> > > > > > > > > + continue;
> > > > > > > > > + if (connector->tile_h_loc == 
> > > > > > > > > connector->num_h_tile - 1 &&
> > > > > > > > > + connector->tile_v_loc == 
> > > > > > > > > connector->num_v_tile - 1)
> > > > > > > > > + continue;
> > > > > > > > > + crtc_state->master_crtc = NULL;
> > > > > > > > > + tile_group_id = 

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-13 Thread Ville Syrjälä
On Thu, Jun 13, 2019 at 09:41:42AM -0700, Manasi Navare wrote:
> On Thu, Jun 13, 2019 at 12:10:57PM +0300, Ville Syrjälä wrote:
> > On Wed, Jun 12, 2019 at 01:59:52PM -0700, Manasi Navare wrote:
> > > On Wed, Jun 12, 2019 at 10:30:55PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Jun 12, 2019 at 12:11:02PM -0700, Manasi Navare wrote:
> > > > > On Wed, Jun 12, 2019 at 10:04:26PM +0300, Ville Syrjälä wrote:
> > > > > > On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst wrote:
> > > > > > > Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > > > > > > > In case of tiled displays when the two tiles are sent across 
> > > > > > > > two CRTCs
> > > > > > > > over two separate DP SST connectors, we need a mechanism to 
> > > > > > > > synchronize
> > > > > > > > the two CRTCs and their corresponding transcoders.
> > > > > > > > So use the master-slave mode where there is one master 
> > > > > > > > corresponding
> > > > > > > > to last horizontal and vertical tile that needs to be genlocked 
> > > > > > > > with
> > > > > > > > all other slave tiles.
> > > > > > > > This patch identifies saves the master CRTC pointer in all the 
> > > > > > > > slave
> > > > > > > > CRTC states. This pointer is needed to select the master 
> > > > > > > > CRTC/transcoder
> > > > > > > > while configuring transcoder port sync for the corresponding 
> > > > > > > > slaves.
> > > > > > > >
> > > > > > > > v2:
> > > > > > > > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > > > > > > > * Use slave_bitmask to save associated slaves in master crtc 
> > > > > > > > state (Ville)
> > > > > > > >
> > > > > > > > Cc: Daniel Vetter 
> > > > > > > > Cc: Ville Syrjälä 
> > > > > > > > Cc: Maarten Lankhorst 
> > > > > > > > Cc: Matt Roper 
> > > > > > > > Signed-off-by: Manasi Navare 
> > > > > > > > ---
> > > > > > > >  drivers/gpu/drm/i915/intel_display.c | 89 
> > > > > > > > 
> > > > > > > >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > > > > > > >  2 files changed, 95 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > index b276345779e6..92dea2231499 100644
> > > > > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > > > > @@ -11316,6 +11316,86 @@ static int 
> > > > > > > > icl_check_nv12_planes(struct intel_crtc_state *crtc_state)
> > > > > > > > return 0;
> > > > > > > >  }
> > > > > > > >  
> > > > > > > > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > > > > > > > +struct intel_crtc_state 
> > > > > > > > *crtc_state,
> > > > > > > > +struct drm_atomic_state *state)
> > > > > > > > +{
> > > > > > > > +   struct drm_i915_private *dev_priv = 
> > > > > > > > to_i915(crtc_state->base.crtc->dev);
> > > > > > > > +   struct drm_connector *master_connector, *connector;
> > > > > > > > +   struct drm_connector_state *connector_state;
> > > > > > > > +   struct drm_connector_list_iter conn_iter;
> > > > > > > > +   struct drm_crtc *master_crtc = NULL;
> > > > > > > > +   struct drm_crtc_state *master_crtc_state;
> > > > > > > > +   int i, tile_group_id;
> > > > > > > > +
> > > > > > > > +   if (INTEL_GEN(dev_priv) < 11)
> > > > > > > > +   return 0;
> > > > > > > > +
> > > > > > > > +   /*
> > > > > > > > +* In case of tiled displays there could be one or more 
> > > > > > > > slaves but there is
> > > > > > > > +* only one master. Lets make the CRTC used by the 
> > > > > > > > connector corresponding
> > > > > > > > +* to the last horizonal and last vertical tile a 
> > > > > > > > master/genlock CRTC.
> > > > > > > > +* All the other CRTCs corresponding to other tiles of 
> > > > > > > > the same Tile group
> > > > > > > > +* are the slave CRTCs and hold a pointer to their 
> > > > > > > > genlock CRTC.
> > > > > > > > +*/
> > > > > > > > +   for_each_new_connector_in_state(state, connector, 
> > > > > > > > connector_state, i) {
> > > > > > > > +   if (connector_state->crtc != crtc)
> > > > > > > > +   continue;
> > > > > > > > +   if (!connector->has_tile)
> > > > > > > > +   continue;
> > > > > > > > +   if (connector->tile_h_loc == 
> > > > > > > > connector->num_h_tile - 1 &&
> > > > > > > > +   connector->tile_v_loc == 
> > > > > > > > connector->num_v_tile - 1)
> > > > > > > > +   continue;
> > > > > > > > +   crtc_state->master_crtc = NULL;
> > > > > > > > +   tile_group_id = connector->tile_group->id;
> > > > > > > > +   drm_connector_list_iter_begin(_priv->drm, 
> > > > > > > > _iter);
> > > > > > > > +   drm_for_each_connector_iter(master_connector, 
> > > > 

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-13 Thread Manasi Navare
On Thu, Jun 13, 2019 at 12:10:57PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 12, 2019 at 01:59:52PM -0700, Manasi Navare wrote:
> > On Wed, Jun 12, 2019 at 10:30:55PM +0300, Ville Syrjälä wrote:
> > > On Wed, Jun 12, 2019 at 12:11:02PM -0700, Manasi Navare wrote:
> > > > On Wed, Jun 12, 2019 at 10:04:26PM +0300, Ville Syrjälä wrote:
> > > > > On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst wrote:
> > > > > > Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > > > > > > In case of tiled displays when the two tiles are sent across two 
> > > > > > > CRTCs
> > > > > > > over two separate DP SST connectors, we need a mechanism to 
> > > > > > > synchronize
> > > > > > > the two CRTCs and their corresponding transcoders.
> > > > > > > So use the master-slave mode where there is one master 
> > > > > > > corresponding
> > > > > > > to last horizontal and vertical tile that needs to be genlocked 
> > > > > > > with
> > > > > > > all other slave tiles.
> > > > > > > This patch identifies saves the master CRTC pointer in all the 
> > > > > > > slave
> > > > > > > CRTC states. This pointer is needed to select the master 
> > > > > > > CRTC/transcoder
> > > > > > > while configuring transcoder port sync for the corresponding 
> > > > > > > slaves.
> > > > > > >
> > > > > > > v2:
> > > > > > > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > > > > > > * Use slave_bitmask to save associated slaves in master crtc 
> > > > > > > state (Ville)
> > > > > > >
> > > > > > > Cc: Daniel Vetter 
> > > > > > > Cc: Ville Syrjälä 
> > > > > > > Cc: Maarten Lankhorst 
> > > > > > > Cc: Matt Roper 
> > > > > > > Signed-off-by: Manasi Navare 
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/intel_display.c | 89 
> > > > > > > 
> > > > > > >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > > > > > >  2 files changed, 95 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > > > index b276345779e6..92dea2231499 100644
> > > > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > > > @@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
> > > > > > > intel_crtc_state *crtc_state)
> > > > > > >   return 0;
> > > > > > >  }
> > > > > > >  
> > > > > > > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > > > > > > +  struct intel_crtc_state *crtc_state,
> > > > > > > +  struct drm_atomic_state *state)
> > > > > > > +{
> > > > > > > + struct drm_i915_private *dev_priv = 
> > > > > > > to_i915(crtc_state->base.crtc->dev);
> > > > > > > + struct drm_connector *master_connector, *connector;
> > > > > > > + struct drm_connector_state *connector_state;
> > > > > > > + struct drm_connector_list_iter conn_iter;
> > > > > > > + struct drm_crtc *master_crtc = NULL;
> > > > > > > + struct drm_crtc_state *master_crtc_state;
> > > > > > > + int i, tile_group_id;
> > > > > > > +
> > > > > > > + if (INTEL_GEN(dev_priv) < 11)
> > > > > > > + return 0;
> > > > > > > +
> > > > > > > + /*
> > > > > > > +  * In case of tiled displays there could be one or more slaves 
> > > > > > > but there is
> > > > > > > +  * only one master. Lets make the CRTC used by the connector 
> > > > > > > corresponding
> > > > > > > +  * to the last horizonal and last vertical tile a 
> > > > > > > master/genlock CRTC.
> > > > > > > +  * All the other CRTCs corresponding to other tiles of the same 
> > > > > > > Tile group
> > > > > > > +  * are the slave CRTCs and hold a pointer to their genlock CRTC.
> > > > > > > +  */
> > > > > > > + for_each_new_connector_in_state(state, connector, 
> > > > > > > connector_state, i) {
> > > > > > > + if (connector_state->crtc != crtc)
> > > > > > > + continue;
> > > > > > > + if (!connector->has_tile)
> > > > > > > + continue;
> > > > > > > + if (connector->tile_h_loc == connector->num_h_tile - 1 
> > > > > > > &&
> > > > > > > + connector->tile_v_loc == connector->num_v_tile - 1)
> > > > > > > + continue;
> > > > > > > + crtc_state->master_crtc = NULL;
> > > > > > > + tile_group_id = connector->tile_group->id;
> > > > > > > + drm_connector_list_iter_begin(_priv->drm, 
> > > > > > > _iter);
> > > > > > > + drm_for_each_connector_iter(master_connector, 
> > > > > > > _iter) {
> > > > > > > + struct drm_connector_state *master_conn_state = 
> > > > > > > NULL;
> > > > > > > +
> > > > > > > + if (!master_connector->has_tile)
> > > > > > > + continue;
> > > > > > > + if (master_connector->tile_h_loc != 
> > > > > > > master_connector->num_h_tile - 1 ||
> > > > > > > + master_connector->tile_v_loc != 
> > > > > > > master_connector->num_v_tile - 1)
> 

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-13 Thread Ville Syrjälä
On Wed, Jun 12, 2019 at 01:59:52PM -0700, Manasi Navare wrote:
> On Wed, Jun 12, 2019 at 10:30:55PM +0300, Ville Syrjälä wrote:
> > On Wed, Jun 12, 2019 at 12:11:02PM -0700, Manasi Navare wrote:
> > > On Wed, Jun 12, 2019 at 10:04:26PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst wrote:
> > > > > Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > > > > > In case of tiled displays when the two tiles are sent across two 
> > > > > > CRTCs
> > > > > > over two separate DP SST connectors, we need a mechanism to 
> > > > > > synchronize
> > > > > > the two CRTCs and their corresponding transcoders.
> > > > > > So use the master-slave mode where there is one master corresponding
> > > > > > to last horizontal and vertical tile that needs to be genlocked with
> > > > > > all other slave tiles.
> > > > > > This patch identifies saves the master CRTC pointer in all the slave
> > > > > > CRTC states. This pointer is needed to select the master 
> > > > > > CRTC/transcoder
> > > > > > while configuring transcoder port sync for the corresponding slaves.
> > > > > >
> > > > > > v2:
> > > > > > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > > > > > * Use slave_bitmask to save associated slaves in master crtc state 
> > > > > > (Ville)
> > > > > >
> > > > > > Cc: Daniel Vetter 
> > > > > > Cc: Ville Syrjälä 
> > > > > > Cc: Maarten Lankhorst 
> > > > > > Cc: Matt Roper 
> > > > > > Signed-off-by: Manasi Navare 
> > > > > > ---
> > > > > >  drivers/gpu/drm/i915/intel_display.c | 89 
> > > > > > 
> > > > > >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > > > > >  2 files changed, 95 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > > index b276345779e6..92dea2231499 100644
> > > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > > @@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
> > > > > > intel_crtc_state *crtc_state)
> > > > > > return 0;
> > > > > >  }
> > > > > >  
> > > > > > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > > > > > +struct intel_crtc_state *crtc_state,
> > > > > > +struct drm_atomic_state *state)
> > > > > > +{
> > > > > > +   struct drm_i915_private *dev_priv = 
> > > > > > to_i915(crtc_state->base.crtc->dev);
> > > > > > +   struct drm_connector *master_connector, *connector;
> > > > > > +   struct drm_connector_state *connector_state;
> > > > > > +   struct drm_connector_list_iter conn_iter;
> > > > > > +   struct drm_crtc *master_crtc = NULL;
> > > > > > +   struct drm_crtc_state *master_crtc_state;
> > > > > > +   int i, tile_group_id;
> > > > > > +
> > > > > > +   if (INTEL_GEN(dev_priv) < 11)
> > > > > > +   return 0;
> > > > > > +
> > > > > > +   /*
> > > > > > +* In case of tiled displays there could be one or more slaves 
> > > > > > but there is
> > > > > > +* only one master. Lets make the CRTC used by the connector 
> > > > > > corresponding
> > > > > > +* to the last horizonal and last vertical tile a 
> > > > > > master/genlock CRTC.
> > > > > > +* All the other CRTCs corresponding to other tiles of the same 
> > > > > > Tile group
> > > > > > +* are the slave CRTCs and hold a pointer to their genlock CRTC.
> > > > > > +*/
> > > > > > +   for_each_new_connector_in_state(state, connector, 
> > > > > > connector_state, i) {
> > > > > > +   if (connector_state->crtc != crtc)
> > > > > > +   continue;
> > > > > > +   if (!connector->has_tile)
> > > > > > +   continue;
> > > > > > +   if (connector->tile_h_loc == connector->num_h_tile - 1 
> > > > > > &&
> > > > > > +   connector->tile_v_loc == connector->num_v_tile - 1)
> > > > > > +   continue;
> > > > > > +   crtc_state->master_crtc = NULL;
> > > > > > +   tile_group_id = connector->tile_group->id;
> > > > > > +   drm_connector_list_iter_begin(_priv->drm, 
> > > > > > _iter);
> > > > > > +   drm_for_each_connector_iter(master_connector, 
> > > > > > _iter) {
> > > > > > +   struct drm_connector_state *master_conn_state = 
> > > > > > NULL;
> > > > > > +
> > > > > > +   if (!master_connector->has_tile)
> > > > > > +   continue;
> > > > > > +   if (master_connector->tile_h_loc != 
> > > > > > master_connector->num_h_tile - 1 ||
> > > > > > +   master_connector->tile_v_loc != 
> > > > > > master_connector->num_v_tile - 1)
> > > > > > +   continue;
> > > > > > +   if (master_connector->tile_group->id != 
> > > > > > tile_group_id)
> > > > > > +   continue;
> > > > > > +
> > > > > > +   

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-12 Thread Manasi Navare
On Wed, Jun 12, 2019 at 10:30:55PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 12, 2019 at 12:11:02PM -0700, Manasi Navare wrote:
> > On Wed, Jun 12, 2019 at 10:04:26PM +0300, Ville Syrjälä wrote:
> > > On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst wrote:
> > > > Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > > > > In case of tiled displays when the two tiles are sent across two CRTCs
> > > > > over two separate DP SST connectors, we need a mechanism to 
> > > > > synchronize
> > > > > the two CRTCs and their corresponding transcoders.
> > > > > So use the master-slave mode where there is one master corresponding
> > > > > to last horizontal and vertical tile that needs to be genlocked with
> > > > > all other slave tiles.
> > > > > This patch identifies saves the master CRTC pointer in all the slave
> > > > > CRTC states. This pointer is needed to select the master 
> > > > > CRTC/transcoder
> > > > > while configuring transcoder port sync for the corresponding slaves.
> > > > >
> > > > > v2:
> > > > > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > > > > * Use slave_bitmask to save associated slaves in master crtc state 
> > > > > (Ville)
> > > > >
> > > > > Cc: Daniel Vetter 
> > > > > Cc: Ville Syrjälä 
> > > > > Cc: Maarten Lankhorst 
> > > > > Cc: Matt Roper 
> > > > > Signed-off-by: Manasi Navare 
> > > > > ---
> > > > >  drivers/gpu/drm/i915/intel_display.c | 89 
> > > > > 
> > > > >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > > > >  2 files changed, 95 insertions(+)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > > index b276345779e6..92dea2231499 100644
> > > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > > @@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
> > > > > intel_crtc_state *crtc_state)
> > > > >   return 0;
> > > > >  }
> > > > >  
> > > > > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > > > > +  struct intel_crtc_state *crtc_state,
> > > > > +  struct drm_atomic_state *state)
> > > > > +{
> > > > > + struct drm_i915_private *dev_priv = 
> > > > > to_i915(crtc_state->base.crtc->dev);
> > > > > + struct drm_connector *master_connector, *connector;
> > > > > + struct drm_connector_state *connector_state;
> > > > > + struct drm_connector_list_iter conn_iter;
> > > > > + struct drm_crtc *master_crtc = NULL;
> > > > > + struct drm_crtc_state *master_crtc_state;
> > > > > + int i, tile_group_id;
> > > > > +
> > > > > + if (INTEL_GEN(dev_priv) < 11)
> > > > > + return 0;
> > > > > +
> > > > > + /*
> > > > > +  * In case of tiled displays there could be one or more slaves 
> > > > > but there is
> > > > > +  * only one master. Lets make the CRTC used by the connector 
> > > > > corresponding
> > > > > +  * to the last horizonal and last vertical tile a 
> > > > > master/genlock CRTC.
> > > > > +  * All the other CRTCs corresponding to other tiles of the same 
> > > > > Tile group
> > > > > +  * are the slave CRTCs and hold a pointer to their genlock CRTC.
> > > > > +  */
> > > > > + for_each_new_connector_in_state(state, connector, 
> > > > > connector_state, i) {
> > > > > + if (connector_state->crtc != crtc)
> > > > > + continue;
> > > > > + if (!connector->has_tile)
> > > > > + continue;
> > > > > + if (connector->tile_h_loc == connector->num_h_tile - 1 
> > > > > &&
> > > > > + connector->tile_v_loc == connector->num_v_tile - 1)
> > > > > + continue;
> > > > > + crtc_state->master_crtc = NULL;
> > > > > + tile_group_id = connector->tile_group->id;
> > > > > + drm_connector_list_iter_begin(_priv->drm, 
> > > > > _iter);
> > > > > + drm_for_each_connector_iter(master_connector, 
> > > > > _iter) {
> > > > > + struct drm_connector_state *master_conn_state = 
> > > > > NULL;
> > > > > +
> > > > > + if (!master_connector->has_tile)
> > > > > + continue;
> > > > > + if (master_connector->tile_h_loc != 
> > > > > master_connector->num_h_tile - 1 ||
> > > > > + master_connector->tile_v_loc != 
> > > > > master_connector->num_v_tile - 1)
> > > > > + continue;
> > > > > + if (master_connector->tile_group->id != 
> > > > > tile_group_id)
> > > > > + continue;
> > > > > +
> > > > > + master_conn_state = 
> > > > > drm_atomic_get_connector_state(state,
> > > > > + 
> > > > >master_connector);
> > > > > +  

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-12 Thread Ville Syrjälä
On Wed, Jun 12, 2019 at 12:11:02PM -0700, Manasi Navare wrote:
> On Wed, Jun 12, 2019 at 10:04:26PM +0300, Ville Syrjälä wrote:
> > On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst wrote:
> > > Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > > > In case of tiled displays when the two tiles are sent across two CRTCs
> > > > over two separate DP SST connectors, we need a mechanism to synchronize
> > > > the two CRTCs and their corresponding transcoders.
> > > > So use the master-slave mode where there is one master corresponding
> > > > to last horizontal and vertical tile that needs to be genlocked with
> > > > all other slave tiles.
> > > > This patch identifies saves the master CRTC pointer in all the slave
> > > > CRTC states. This pointer is needed to select the master CRTC/transcoder
> > > > while configuring transcoder port sync for the corresponding slaves.
> > > >
> > > > v2:
> > > > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > > > * Use slave_bitmask to save associated slaves in master crtc state 
> > > > (Ville)
> > > >
> > > > Cc: Daniel Vetter 
> > > > Cc: Ville Syrjälä 
> > > > Cc: Maarten Lankhorst 
> > > > Cc: Matt Roper 
> > > > Signed-off-by: Manasi Navare 
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_display.c | 89 
> > > >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > > >  2 files changed, 95 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > > b/drivers/gpu/drm/i915/intel_display.c
> > > > index b276345779e6..92dea2231499 100644
> > > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > > @@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
> > > > intel_crtc_state *crtc_state)
> > > > return 0;
> > > >  }
> > > >  
> > > > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > > > +struct intel_crtc_state *crtc_state,
> > > > +struct drm_atomic_state *state)
> > > > +{
> > > > +   struct drm_i915_private *dev_priv = 
> > > > to_i915(crtc_state->base.crtc->dev);
> > > > +   struct drm_connector *master_connector, *connector;
> > > > +   struct drm_connector_state *connector_state;
> > > > +   struct drm_connector_list_iter conn_iter;
> > > > +   struct drm_crtc *master_crtc = NULL;
> > > > +   struct drm_crtc_state *master_crtc_state;
> > > > +   int i, tile_group_id;
> > > > +
> > > > +   if (INTEL_GEN(dev_priv) < 11)
> > > > +   return 0;
> > > > +
> > > > +   /*
> > > > +* In case of tiled displays there could be one or more slaves 
> > > > but there is
> > > > +* only one master. Lets make the CRTC used by the connector 
> > > > corresponding
> > > > +* to the last horizonal and last vertical tile a 
> > > > master/genlock CRTC.
> > > > +* All the other CRTCs corresponding to other tiles of the same 
> > > > Tile group
> > > > +* are the slave CRTCs and hold a pointer to their genlock CRTC.
> > > > +*/
> > > > +   for_each_new_connector_in_state(state, connector, 
> > > > connector_state, i) {
> > > > +   if (connector_state->crtc != crtc)
> > > > +   continue;
> > > > +   if (!connector->has_tile)
> > > > +   continue;
> > > > +   if (connector->tile_h_loc == connector->num_h_tile - 1 
> > > > &&
> > > > +   connector->tile_v_loc == connector->num_v_tile - 1)
> > > > +   continue;
> > > > +   crtc_state->master_crtc = NULL;
> > > > +   tile_group_id = connector->tile_group->id;
> > > > +   drm_connector_list_iter_begin(_priv->drm, 
> > > > _iter);
> > > > +   drm_for_each_connector_iter(master_connector, 
> > > > _iter) {
> > > > +   struct drm_connector_state *master_conn_state = 
> > > > NULL;
> > > > +
> > > > +   if (!master_connector->has_tile)
> > > > +   continue;
> > > > +   if (master_connector->tile_h_loc != 
> > > > master_connector->num_h_tile - 1 ||
> > > > +   master_connector->tile_v_loc != 
> > > > master_connector->num_v_tile - 1)
> > > > +   continue;
> > > > +   if (master_connector->tile_group->id != 
> > > > tile_group_id)
> > > > +   continue;
> > > > +
> > > > +   master_conn_state = 
> > > > drm_atomic_get_connector_state(state,
> > > > +   
> > > >master_connector);
> > > > +   if (IS_ERR(master_conn_state)) {
> > > > +   drm_connector_list_iter_end(_iter);
> > > > +   return PTR_ERR(master_conn_state);
> > > > +  

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-12 Thread Manasi Navare
On Wed, Jun 12, 2019 at 10:04:26PM +0300, Ville Syrjälä wrote:
> On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst wrote:
> > Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > > In case of tiled displays when the two tiles are sent across two CRTCs
> > > over two separate DP SST connectors, we need a mechanism to synchronize
> > > the two CRTCs and their corresponding transcoders.
> > > So use the master-slave mode where there is one master corresponding
> > > to last horizontal and vertical tile that needs to be genlocked with
> > > all other slave tiles.
> > > This patch identifies saves the master CRTC pointer in all the slave
> > > CRTC states. This pointer is needed to select the master CRTC/transcoder
> > > while configuring transcoder port sync for the corresponding slaves.
> > >
> > > v2:
> > > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > > * Use slave_bitmask to save associated slaves in master crtc state (Ville)
> > >
> > > Cc: Daniel Vetter 
> > > Cc: Ville Syrjälä 
> > > Cc: Maarten Lankhorst 
> > > Cc: Matt Roper 
> > > Signed-off-by: Manasi Navare 
> > > ---
> > >  drivers/gpu/drm/i915/intel_display.c | 89 
> > >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > >  2 files changed, 95 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index b276345779e6..92dea2231499 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
> > > intel_crtc_state *crtc_state)
> > >   return 0;
> > >  }
> > >  
> > > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > > +  struct intel_crtc_state *crtc_state,
> > > +  struct drm_atomic_state *state)
> > > +{
> > > + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > > + struct drm_connector *master_connector, *connector;
> > > + struct drm_connector_state *connector_state;
> > > + struct drm_connector_list_iter conn_iter;
> > > + struct drm_crtc *master_crtc = NULL;
> > > + struct drm_crtc_state *master_crtc_state;
> > > + int i, tile_group_id;
> > > +
> > > + if (INTEL_GEN(dev_priv) < 11)
> > > + return 0;
> > > +
> > > + /*
> > > +  * In case of tiled displays there could be one or more slaves but 
> > > there is
> > > +  * only one master. Lets make the CRTC used by the connector 
> > > corresponding
> > > +  * to the last horizonal and last vertical tile a master/genlock CRTC.
> > > +  * All the other CRTCs corresponding to other tiles of the same Tile 
> > > group
> > > +  * are the slave CRTCs and hold a pointer to their genlock CRTC.
> > > +  */
> > > + for_each_new_connector_in_state(state, connector, connector_state, i) {
> > > + if (connector_state->crtc != crtc)
> > > + continue;
> > > + if (!connector->has_tile)
> > > + continue;
> > > + if (connector->tile_h_loc == connector->num_h_tile - 1 &&
> > > + connector->tile_v_loc == connector->num_v_tile - 1)
> > > + continue;
> > > + crtc_state->master_crtc = NULL;
> > > + tile_group_id = connector->tile_group->id;
> > > + drm_connector_list_iter_begin(_priv->drm, _iter);
> > > + drm_for_each_connector_iter(master_connector, _iter) {
> > > + struct drm_connector_state *master_conn_state = NULL;
> > > +
> > > + if (!master_connector->has_tile)
> > > + continue;
> > > + if (master_connector->tile_h_loc != 
> > > master_connector->num_h_tile - 1 ||
> > > + master_connector->tile_v_loc != 
> > > master_connector->num_v_tile - 1)
> > > + continue;
> > > + if (master_connector->tile_group->id != tile_group_id)
> > > + continue;
> > > +
> > > + master_conn_state = 
> > > drm_atomic_get_connector_state(state,
> > > +
> > > master_connector);
> > > + if (IS_ERR(master_conn_state)) {
> > > + drm_connector_list_iter_end(_iter);
> > > + return PTR_ERR(master_conn_state);
> > > + }
> > > + if (master_conn_state->crtc) {
> > > + master_crtc = master_conn_state->crtc;
> > > + break;
> > > + }
> > > + }
> > > + drm_connector_list_iter_end(_iter);
> > > +
> > > + if (!master_crtc) {
> > > + DRM_DEBUG_KMS("Could not add Master CRTC for Slave CRTC 
> > > %d\n",
> > > +   connector_state->crtc->base.id);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + master_crtc_state = 

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-12 Thread Ville Syrjälä
On Wed, Jun 12, 2019 at 11:39:03AM +0200, Maarten Lankhorst wrote:
> Op 23-04-2019 om 17:48 schreef Manasi Navare:
> > In case of tiled displays when the two tiles are sent across two CRTCs
> > over two separate DP SST connectors, we need a mechanism to synchronize
> > the two CRTCs and their corresponding transcoders.
> > So use the master-slave mode where there is one master corresponding
> > to last horizontal and vertical tile that needs to be genlocked with
> > all other slave tiles.
> > This patch identifies saves the master CRTC pointer in all the slave
> > CRTC states. This pointer is needed to select the master CRTC/transcoder
> > while configuring transcoder port sync for the corresponding slaves.
> >
> > v2:
> > * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> > * Use slave_bitmask to save associated slaves in master crtc state (Ville)
> >
> > Cc: Daniel Vetter 
> > Cc: Ville Syrjälä 
> > Cc: Maarten Lankhorst 
> > Cc: Matt Roper 
> > Signed-off-by: Manasi Navare 
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 89 
> >  drivers/gpu/drm/i915/intel_drv.h |  6 ++
> >  2 files changed, 95 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c 
> > b/drivers/gpu/drm/i915/intel_display.c
> > index b276345779e6..92dea2231499 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
> > intel_crtc_state *crtc_state)
> > return 0;
> >  }
> >  
> > +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> > +struct intel_crtc_state *crtc_state,
> > +struct drm_atomic_state *state)
> > +{
> > +   struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > +   struct drm_connector *master_connector, *connector;
> > +   struct drm_connector_state *connector_state;
> > +   struct drm_connector_list_iter conn_iter;
> > +   struct drm_crtc *master_crtc = NULL;
> > +   struct drm_crtc_state *master_crtc_state;
> > +   int i, tile_group_id;
> > +
> > +   if (INTEL_GEN(dev_priv) < 11)
> > +   return 0;
> > +
> > +   /*
> > +* In case of tiled displays there could be one or more slaves but 
> > there is
> > +* only one master. Lets make the CRTC used by the connector 
> > corresponding
> > +* to the last horizonal and last vertical tile a master/genlock CRTC.
> > +* All the other CRTCs corresponding to other tiles of the same Tile 
> > group
> > +* are the slave CRTCs and hold a pointer to their genlock CRTC.
> > +*/
> > +   for_each_new_connector_in_state(state, connector, connector_state, i) {
> > +   if (connector_state->crtc != crtc)
> > +   continue;
> > +   if (!connector->has_tile)
> > +   continue;
> > +   if (connector->tile_h_loc == connector->num_h_tile - 1 &&
> > +   connector->tile_v_loc == connector->num_v_tile - 1)
> > +   continue;
> > +   crtc_state->master_crtc = NULL;
> > +   tile_group_id = connector->tile_group->id;
> > +   drm_connector_list_iter_begin(_priv->drm, _iter);
> > +   drm_for_each_connector_iter(master_connector, _iter) {
> > +   struct drm_connector_state *master_conn_state = NULL;
> > +
> > +   if (!master_connector->has_tile)
> > +   continue;
> > +   if (master_connector->tile_h_loc != 
> > master_connector->num_h_tile - 1 ||
> > +   master_connector->tile_v_loc != 
> > master_connector->num_v_tile - 1)
> > +   continue;
> > +   if (master_connector->tile_group->id != tile_group_id)
> > +   continue;
> > +
> > +   master_conn_state = 
> > drm_atomic_get_connector_state(state,
> > +  
> > master_connector);
> > +   if (IS_ERR(master_conn_state)) {
> > +   drm_connector_list_iter_end(_iter);
> > +   return PTR_ERR(master_conn_state);
> > +   }
> > +   if (master_conn_state->crtc) {
> > +   master_crtc = master_conn_state->crtc;
> > +   break;
> > +   }
> > +   }
> > +   drm_connector_list_iter_end(_iter);
> > +
> > +   if (!master_crtc) {
> > +   DRM_DEBUG_KMS("Could not add Master CRTC for Slave CRTC 
> > %d\n",
> > + connector_state->crtc->base.id);
> > +   return -EINVAL;
> > +   }
> > +
> > +   master_crtc_state = drm_atomic_get_crtc_state(state,
> > + master_crtc);
> > +   if (IS_ERR(master_crtc_state))
> > +   

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-12 Thread Maarten Lankhorst
Op 23-04-2019 om 17:48 schreef Manasi Navare:
> In case of tiled displays when the two tiles are sent across two CRTCs
> over two separate DP SST connectors, we need a mechanism to synchronize
> the two CRTCs and their corresponding transcoders.
> So use the master-slave mode where there is one master corresponding
> to last horizontal and vertical tile that needs to be genlocked with
> all other slave tiles.
> This patch identifies saves the master CRTC pointer in all the slave
> CRTC states. This pointer is needed to select the master CRTC/transcoder
> while configuring transcoder port sync for the corresponding slaves.
>
> v2:
> * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> * Use slave_bitmask to save associated slaves in master crtc state (Ville)
>
> Cc: Daniel Vetter 
> Cc: Ville Syrjälä 
> Cc: Maarten Lankhorst 
> Cc: Matt Roper 
> Signed-off-by: Manasi Navare 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 89 
>  drivers/gpu/drm/i915/intel_drv.h |  6 ++
>  2 files changed, 95 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index b276345779e6..92dea2231499 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
> intel_crtc_state *crtc_state)
>   return 0;
>  }
>  
> +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> +  struct intel_crtc_state *crtc_state,
> +  struct drm_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> + struct drm_connector *master_connector, *connector;
> + struct drm_connector_state *connector_state;
> + struct drm_connector_list_iter conn_iter;
> + struct drm_crtc *master_crtc = NULL;
> + struct drm_crtc_state *master_crtc_state;
> + int i, tile_group_id;
> +
> + if (INTEL_GEN(dev_priv) < 11)
> + return 0;
> +
> + /*
> +  * In case of tiled displays there could be one or more slaves but 
> there is
> +  * only one master. Lets make the CRTC used by the connector 
> corresponding
> +  * to the last horizonal and last vertical tile a master/genlock CRTC.
> +  * All the other CRTCs corresponding to other tiles of the same Tile 
> group
> +  * are the slave CRTCs and hold a pointer to their genlock CRTC.
> +  */
> + for_each_new_connector_in_state(state, connector, connector_state, i) {
> + if (connector_state->crtc != crtc)
> + continue;
> + if (!connector->has_tile)
> + continue;
> + if (connector->tile_h_loc == connector->num_h_tile - 1 &&
> + connector->tile_v_loc == connector->num_v_tile - 1)
> + continue;
> + crtc_state->master_crtc = NULL;
> + tile_group_id = connector->tile_group->id;
> + drm_connector_list_iter_begin(_priv->drm, _iter);
> + drm_for_each_connector_iter(master_connector, _iter) {
> + struct drm_connector_state *master_conn_state = NULL;
> +
> + if (!master_connector->has_tile)
> + continue;
> + if (master_connector->tile_h_loc != 
> master_connector->num_h_tile - 1 ||
> + master_connector->tile_v_loc != 
> master_connector->num_v_tile - 1)
> + continue;
> + if (master_connector->tile_group->id != tile_group_id)
> + continue;
> +
> + master_conn_state = 
> drm_atomic_get_connector_state(state,
> +
> master_connector);
> + if (IS_ERR(master_conn_state)) {
> + drm_connector_list_iter_end(_iter);
> + return PTR_ERR(master_conn_state);
> + }
> + if (master_conn_state->crtc) {
> + master_crtc = master_conn_state->crtc;
> + break;
> + }
> + }
> + drm_connector_list_iter_end(_iter);
> +
> + if (!master_crtc) {
> + DRM_DEBUG_KMS("Could not add Master CRTC for Slave CRTC 
> %d\n",
> +   connector_state->crtc->base.id);
> + return -EINVAL;
> + }
> +
> + master_crtc_state = drm_atomic_get_crtc_state(state,
> +   master_crtc);
> + if (IS_ERR(master_crtc_state))
> + return PTR_ERR(master_crtc_state);
> +
> + crtc_state->master_crtc = to_intel_crtc(master_crtc);
> + 

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-06-11 Thread Manasi Navare
Need some suggestions on what could be stored in slave_crtc_state to point to 
the master,
See my comments/questions inline

On Tue, Apr 23, 2019 at 08:48:58AM -0700, Manasi Navare wrote:
> In case of tiled displays when the two tiles are sent across two CRTCs
> over two separate DP SST connectors, we need a mechanism to synchronize
> the two CRTCs and their corresponding transcoders.
> So use the master-slave mode where there is one master corresponding
> to last horizontal and vertical tile that needs to be genlocked with
> all other slave tiles.
> This patch identifies saves the master CRTC pointer in all the slave
> CRTC states. This pointer is needed to select the master CRTC/transcoder
> while configuring transcoder port sync for the corresponding slaves.
> 
> v2:
> * Move this to intel_mode_set_pipe_config(Jani N, Ville)
> * Use slave_bitmask to save associated slaves in master crtc state (Ville)
> 
> Cc: Daniel Vetter 
> Cc: Ville Syrjälä 
> Cc: Maarten Lankhorst 
> Cc: Matt Roper 
> Signed-off-by: Manasi Navare 
> ---
>  drivers/gpu/drm/i915/intel_display.c | 89 
>  drivers/gpu/drm/i915/intel_drv.h |  6 ++
>  2 files changed, 95 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c 
> b/drivers/gpu/drm/i915/intel_display.c
> index b276345779e6..92dea2231499 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
> intel_crtc_state *crtc_state)
>   return 0;
>  }
>  
> +static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> +  struct intel_crtc_state *crtc_state,
> +  struct drm_atomic_state *state)
> +{
> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> + struct drm_connector *master_connector, *connector;
> + struct drm_connector_state *connector_state;
> + struct drm_connector_list_iter conn_iter;
> + struct drm_crtc *master_crtc = NULL;
> + struct drm_crtc_state *master_crtc_state;
> + int i, tile_group_id;
> +
> + if (INTEL_GEN(dev_priv) < 11)
> + return 0;
> +
> + /*
> +  * In case of tiled displays there could be one or more slaves but 
> there is
> +  * only one master. Lets make the CRTC used by the connector 
> corresponding
> +  * to the last horizonal and last vertical tile a master/genlock CRTC.
> +  * All the other CRTCs corresponding to other tiles of the same Tile 
> group
> +  * are the slave CRTCs and hold a pointer to their genlock CRTC.
> +  */
> + for_each_new_connector_in_state(state, connector, connector_state, i) {
> + if (connector_state->crtc != crtc)
> + continue;
> + if (!connector->has_tile)
> + continue;
> + if (connector->tile_h_loc == connector->num_h_tile - 1 &&
> + connector->tile_v_loc == connector->num_v_tile - 1)
> + continue;
> + crtc_state->master_crtc = NULL;
> + tile_group_id = connector->tile_group->id;
> + drm_connector_list_iter_begin(_priv->drm, _iter);
> + drm_for_each_connector_iter(master_connector, _iter) {
> + struct drm_connector_state *master_conn_state = NULL;
> +
> + if (!master_connector->has_tile)
> + continue;
> + if (master_connector->tile_h_loc != 
> master_connector->num_h_tile - 1 ||
> + master_connector->tile_v_loc != 
> master_connector->num_v_tile - 1)
> + continue;
> + if (master_connector->tile_group->id != tile_group_id)
> + continue;
> +
> + master_conn_state = 
> drm_atomic_get_connector_state(state,
> +
> master_connector);
> + if (IS_ERR(master_conn_state)) {
> + drm_connector_list_iter_end(_iter);
> + return PTR_ERR(master_conn_state);
> + }
> + if (master_conn_state->crtc) {
> + master_crtc = master_conn_state->crtc;
> + break;
> + }
> + }
> + drm_connector_list_iter_end(_iter);
> +
> + if (!master_crtc) {
> + DRM_DEBUG_KMS("Could not add Master CRTC for Slave CRTC 
> %d\n",
> +   connector_state->crtc->base.id);
> + return -EINVAL;
> + }
> +
> + master_crtc_state = drm_atomic_get_crtc_state(state,
> +   master_crtc);
> + if (IS_ERR(master_crtc_state))
> + return 

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-05-22 Thread Manasi Navare
On Fri, May 03, 2019 at 05:09:20PM -0700, Srivatsa, Anusha wrote:
> 
> 
> >-Original Message-
> >From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
> >Manasi Navare
> >Sent: Tuesday, April 23, 2019 8:49 AM
> >To: intel-gfx@lists.freedesktop.org
> >Cc: Daniel Vetter 
> >Subject: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc 
> >links for
> >Transcoder Port Sync
> >
> >In case of tiled displays when the two tiles are sent across two CRTCs over 
> >two
> >separate DP SST connectors, we need a mechanism to synchronize the two CRTCs
> >and their corresponding transcoders.
> >So use the master-slave mode where there is one master corresponding to last
> >horizontal and vertical tile that needs to be genlocked with all other slave 
> >tiles.
> >This patch identifies saves the master CRTC pointer in all the slave CRTC 
> >states.
> >This pointer is needed to select the master CRTC/transcoder while configuring
> >transcoder port sync for the corresponding slaves.
> >
> >v2:
> >* Move this to intel_mode_set_pipe_config(Jani N, Ville)
> >* Use slave_bitmask to save associated slaves in master crtc state (Ville)
> >
> >Cc: Daniel Vetter 
> >Cc: Ville Syrjälä 
> >Cc: Maarten Lankhorst 
> >Cc: Matt Roper 
> >Signed-off-by: Manasi Navare 
> >---
> > drivers/gpu/drm/i915/intel_display.c | 89 
> > drivers/gpu/drm/i915/intel_drv.h |  6 ++
> > 2 files changed, 95 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_display.c
> >b/drivers/gpu/drm/i915/intel_display.c
> >index b276345779e6..92dea2231499 100644
> >--- a/drivers/gpu/drm/i915/intel_display.c
> >+++ b/drivers/gpu/drm/i915/intel_display.c
> >@@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct
> >intel_crtc_state *crtc_state)
> > return 0;
> > }
> >
> >+static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
> >+ struct intel_crtc_state *crtc_state,
> >+ struct drm_atomic_state *state)
> >+{
> >+struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> >+struct drm_connector *master_connector, *connector;
> >+struct drm_connector_state *connector_state;
> >+struct drm_connector_list_iter conn_iter;
> >+struct drm_crtc *master_crtc = NULL;
> >+struct drm_crtc_state *master_crtc_state;
> >+int i, tile_group_id;
> >+
> >+if (INTEL_GEN(dev_priv) < 11)
> >+return 0;
> >+
> >+/*
> >+ * In case of tiled displays there could be one or more slaves but 
> >there is
> >+ * only one master. Lets make the CRTC used by the connector
> >corresponding
> >+ * to the last horizonal and last vertical tile a master/genlock CRTC.
> >+ * All the other CRTCs corresponding to other tiles of the same Tile 
> >group
> >+ * are the slave CRTCs and hold a pointer to their genlock CRTC.
> >+ */
> >+for_each_new_connector_in_state(state, connector, connector_state, i)
> >{
> >+if (connector_state->crtc != crtc)
> >+continue;
> >+if (!connector->has_tile)
> >+continue;
> >+if (connector->tile_h_loc == connector->num_h_tile - 1 &&
> >+connector->tile_v_loc == connector->num_v_tile - 1)
> >+continue;
> >+crtc_state->master_crtc = NULL;
> >+tile_group_id = connector->tile_group->id;
> >+drm_connector_list_iter_begin(_priv->drm, _iter);
> >+drm_for_each_connector_iter(master_connector, _iter) {
> >+struct drm_connector_state *master_conn_state =
> >NULL;
> >+
> >+if (!master_connector->has_tile)
> >+continue;
> >+if (master_connector->tile_h_loc != master_connector-
> >>num_h_tile - 1 ||
> >+master_connector->tile_v_loc != master_connector-
> >>num_v_tile - 1)
> 
> Will this ever be true? With the checks above (for slaves) we have iterated 
> over all slaves and potentially left with only master/last tile right?
> 
> Anusha 

Actually its the other way around. So like the comment says the last tile is 
the master and all others are slaves,
so here in for_each_connector(), we skip the master and call 
drm_for_each_connector_iter() fo

Re: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-05-03 Thread Srivatsa, Anusha


>-Original Message-
>From: Intel-gfx [mailto:intel-gfx-boun...@lists.freedesktop.org] On Behalf Of
>Manasi Navare
>Sent: Tuesday, April 23, 2019 8:49 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Daniel Vetter 
>Subject: [Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc 
>links for
>Transcoder Port Sync
>
>In case of tiled displays when the two tiles are sent across two CRTCs over two
>separate DP SST connectors, we need a mechanism to synchronize the two CRTCs
>and their corresponding transcoders.
>So use the master-slave mode where there is one master corresponding to last
>horizontal and vertical tile that needs to be genlocked with all other slave 
>tiles.
>This patch identifies saves the master CRTC pointer in all the slave CRTC 
>states.
>This pointer is needed to select the master CRTC/transcoder while configuring
>transcoder port sync for the corresponding slaves.
>
>v2:
>* Move this to intel_mode_set_pipe_config(Jani N, Ville)
>* Use slave_bitmask to save associated slaves in master crtc state (Ville)
>
>Cc: Daniel Vetter 
>Cc: Ville Syrjälä 
>Cc: Maarten Lankhorst 
>Cc: Matt Roper 
>Signed-off-by: Manasi Navare 
>---
> drivers/gpu/drm/i915/intel_display.c | 89 
> drivers/gpu/drm/i915/intel_drv.h |  6 ++
> 2 files changed, 95 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/intel_display.c
>b/drivers/gpu/drm/i915/intel_display.c
>index b276345779e6..92dea2231499 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct
>intel_crtc_state *crtc_state)
>   return 0;
> }
>
>+static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
>+   struct intel_crtc_state *crtc_state,
>+   struct drm_atomic_state *state)
>+{
>+  struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>+  struct drm_connector *master_connector, *connector;
>+  struct drm_connector_state *connector_state;
>+  struct drm_connector_list_iter conn_iter;
>+  struct drm_crtc *master_crtc = NULL;
>+  struct drm_crtc_state *master_crtc_state;
>+  int i, tile_group_id;
>+
>+  if (INTEL_GEN(dev_priv) < 11)
>+  return 0;
>+
>+  /*
>+   * In case of tiled displays there could be one or more slaves but 
>there is
>+   * only one master. Lets make the CRTC used by the connector
>corresponding
>+   * to the last horizonal and last vertical tile a master/genlock CRTC.
>+   * All the other CRTCs corresponding to other tiles of the same Tile 
>group
>+   * are the slave CRTCs and hold a pointer to their genlock CRTC.
>+   */
>+  for_each_new_connector_in_state(state, connector, connector_state, i)
>{
>+  if (connector_state->crtc != crtc)
>+  continue;
>+  if (!connector->has_tile)
>+  continue;
>+  if (connector->tile_h_loc == connector->num_h_tile - 1 &&
>+  connector->tile_v_loc == connector->num_v_tile - 1)
>+  continue;
>+  crtc_state->master_crtc = NULL;
>+  tile_group_id = connector->tile_group->id;
>+  drm_connector_list_iter_begin(_priv->drm, _iter);
>+  drm_for_each_connector_iter(master_connector, _iter) {
>+  struct drm_connector_state *master_conn_state =
>NULL;
>+
>+  if (!master_connector->has_tile)
>+  continue;
>+  if (master_connector->tile_h_loc != master_connector-
>>num_h_tile - 1 ||
>+  master_connector->tile_v_loc != master_connector-
>>num_v_tile - 1)

Will this ever be true? With the checks above (for slaves) we have iterated 
over all slaves and potentially left with only master/last tile right?

Anusha 

>+  continue;
>+  if (master_connector->tile_group->id != tile_group_id)
>+  continue;
>+
>+  master_conn_state =
>drm_atomic_get_connector_state(state,
>+
>master_connector);
>+  if (IS_ERR(master_conn_state)) {
>+  drm_connector_list_iter_end(_iter);
>+  return PTR_ERR(master_conn_state);
>+  }
>+  if (master_conn_state->crtc) {
>+  master_crtc = master_conn_state->crtc;
>+  

[Intel-gfx] [PATCH v2 1/4] drm/i915/icl: Assign Master slave crtc links for Transcoder Port Sync

2019-04-23 Thread Manasi Navare
In case of tiled displays when the two tiles are sent across two CRTCs
over two separate DP SST connectors, we need a mechanism to synchronize
the two CRTCs and their corresponding transcoders.
So use the master-slave mode where there is one master corresponding
to last horizontal and vertical tile that needs to be genlocked with
all other slave tiles.
This patch identifies saves the master CRTC pointer in all the slave
CRTC states. This pointer is needed to select the master CRTC/transcoder
while configuring transcoder port sync for the corresponding slaves.

v2:
* Move this to intel_mode_set_pipe_config(Jani N, Ville)
* Use slave_bitmask to save associated slaves in master crtc state (Ville)

Cc: Daniel Vetter 
Cc: Ville Syrjälä 
Cc: Maarten Lankhorst 
Cc: Matt Roper 
Signed-off-by: Manasi Navare 
---
 drivers/gpu/drm/i915/intel_display.c | 89 
 drivers/gpu/drm/i915/intel_drv.h |  6 ++
 2 files changed, 95 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index b276345779e6..92dea2231499 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11316,6 +11316,86 @@ static int icl_check_nv12_planes(struct 
intel_crtc_state *crtc_state)
return 0;
 }
 
+static int icl_add_genlock_crtcs(struct drm_crtc *crtc,
+struct intel_crtc_state *crtc_state,
+struct drm_atomic_state *state)
+{
+   struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+   struct drm_connector *master_connector, *connector;
+   struct drm_connector_state *connector_state;
+   struct drm_connector_list_iter conn_iter;
+   struct drm_crtc *master_crtc = NULL;
+   struct drm_crtc_state *master_crtc_state;
+   int i, tile_group_id;
+
+   if (INTEL_GEN(dev_priv) < 11)
+   return 0;
+
+   /*
+* In case of tiled displays there could be one or more slaves but 
there is
+* only one master. Lets make the CRTC used by the connector 
corresponding
+* to the last horizonal and last vertical tile a master/genlock CRTC.
+* All the other CRTCs corresponding to other tiles of the same Tile 
group
+* are the slave CRTCs and hold a pointer to their genlock CRTC.
+*/
+   for_each_new_connector_in_state(state, connector, connector_state, i) {
+   if (connector_state->crtc != crtc)
+   continue;
+   if (!connector->has_tile)
+   continue;
+   if (connector->tile_h_loc == connector->num_h_tile - 1 &&
+   connector->tile_v_loc == connector->num_v_tile - 1)
+   continue;
+   crtc_state->master_crtc = NULL;
+   tile_group_id = connector->tile_group->id;
+   drm_connector_list_iter_begin(_priv->drm, _iter);
+   drm_for_each_connector_iter(master_connector, _iter) {
+   struct drm_connector_state *master_conn_state = NULL;
+
+   if (!master_connector->has_tile)
+   continue;
+   if (master_connector->tile_h_loc != 
master_connector->num_h_tile - 1 ||
+   master_connector->tile_v_loc != 
master_connector->num_v_tile - 1)
+   continue;
+   if (master_connector->tile_group->id != tile_group_id)
+   continue;
+
+   master_conn_state = 
drm_atomic_get_connector_state(state,
+  
master_connector);
+   if (IS_ERR(master_conn_state)) {
+   drm_connector_list_iter_end(_iter);
+   return PTR_ERR(master_conn_state);
+   }
+   if (master_conn_state->crtc) {
+   master_crtc = master_conn_state->crtc;
+   break;
+   }
+   }
+   drm_connector_list_iter_end(_iter);
+
+   if (!master_crtc) {
+   DRM_DEBUG_KMS("Could not add Master CRTC for Slave CRTC 
%d\n",
+ connector_state->crtc->base.id);
+   return -EINVAL;
+   }
+
+   master_crtc_state = drm_atomic_get_crtc_state(state,
+ master_crtc);
+   if (IS_ERR(master_crtc_state))
+   return PTR_ERR(master_crtc_state);
+
+   crtc_state->master_crtc = to_intel_crtc(master_crtc);
+   to_intel_crtc_state(master_crtc_state)->trans_port_sync_slaves 
|=
+   BIT(to_intel_crtc(crtc)->pipe);
+   DRM_DEBUG_KMS("Master CRTC = %d added for