| Message ID | 20201008214535.22942-7-manasi.d.navare@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v10,01/11] HAX to make DSC work on the icelake test system | expand |
On Thu, Oct 08, 2020 at 02:45:31PM -0700, Manasi Navare wrote: > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Unfortunately I have no way to test this, but it should be correct > if the bios sets up bigjoiner in a sane way. > > Skip iterating over bigjoiner slaves, only the master has the state we > care about. > > Add the width of the bigjoiner slave to the reconstructed fb. > > Hide the bigjoiner slave to userspace, and double the mode on bigjoiner > master. > > And last, disable bigjoiner slave from primary if reconstruction fails. > > v2: > * Manual Rebase (Manasi) > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display.c | 64 +++++++++++++++++++- > 1 file changed, 62 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index faf8bf757bed..aa981aa4f6a1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -3619,6 +3619,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > struct intel_plane *intel_plane = to_intel_plane(primary); > struct intel_plane_state *intel_state = > to_intel_plane_state(plane_state); > + struct intel_crtc_state *crtc_state = Whitespace fail. Didn't checkpatch complain? > + to_intel_crtc_state(intel_crtc->base.state); > struct drm_framebuffer *fb; > struct i915_vma *vma; > > @@ -3641,7 +3643,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > if (c == &intel_crtc->base) > continue; > > - if (!to_intel_crtc(c)->active) > + if (!to_intel_crtc_state(c->state)->uapi.active) > continue; > > state = to_intel_plane_state(c->primary->state); > @@ -3663,6 +3665,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > * pretend the BIOS never had it enabled. > */ > intel_plane_disable_noatomic(intel_crtc, intel_plane); > + if (crtc_state->bigjoiner) { > + struct intel_crtc *slave = > + crtc_state->bigjoiner_linked_crtc; > + intel_plane_disable_noatomic(slave, to_intel_plane(slave->base.primary)); > + } > > return; > > @@ -10687,6 +10694,7 @@ static void > skl_get_initial_plane_config(struct intel_crtc *crtc, > struct intel_initial_plane_config *plane_config) > { > + struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); > struct drm_device *dev = crtc->base.dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_plane *plane = to_intel_plane(crtc->base.primary); > @@ -10795,6 +10803,18 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > fb->height = ((val >> 16) & 0xffff) + 1; > fb->width = ((val >> 0) & 0xffff) + 1; > > + /* add bigjoiner slave as well, if the fb stretches both */ > + if (crtc_state->bigjoiner) { > + enum pipe bigjoiner_pipe = crtc_state->bigjoiner_linked_crtc->pipe; > + > + if (fb->width == crtc_state->pipe_src_w && > + (intel_de_read(dev_priv, PLANE_SURF(pipe, plane_id)) & 0xfffff000) == plane_config->base) { > + val = intel_de_read(dev_priv, PLANE_SIZE(bigjoiner_pipe, plane_id)); > + fb->height += ((val >> 16) & 0xfff) + 1; > + fb->width += ((val >> 0) & 0x1fff) + 1; This looks wrong. > + } > + } > + > val = intel_de_read(dev_priv, PLANE_STRIDE(pipe, plane_id)); > stride_mult = skl_plane_stride_mult(fb, 0, DRM_MODE_ROTATE_0); > fb->pitches[0] = (val & 0x3ff) * stride_mult; > @@ -18793,7 +18813,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, > > /* Adjust the state of the output pipe according to whether we > * have active connectors/encoders. */ > - if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc)) > + if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) && > + !crtc_state->bigjoiner_slave) > intel_crtc_disable_noatomic(crtc, ctx); > > if (crtc_state->hw.active || HAS_GMCH(dev_priv)) { > @@ -19072,6 +19093,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > struct intel_plane *plane; > int min_cdclk = 0; > > + if (crtc_state->bigjoiner_slave) > + continue; > + > if (crtc_state->hw.active) { > struct drm_display_mode mode; > > @@ -19096,6 +19120,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > mode.hdisplay = crtc_state->pipe_src_w; > mode.vdisplay = crtc_state->pipe_src_h; > > + if (crtc_state->bigjoiner) > + mode.hdisplay *= 2; > + > intel_crtc_compute_pixel_rate(crtc_state); > > intel_crtc_update_active_timings(crtc_state); > @@ -19146,6 +19173,39 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > intel_bw_crtc_update(bw_state, crtc_state); > > intel_pipe_config_sanity_check(dev_priv, crtc_state); > + > + /* discard our incomplete slave state, copy it from master */ Wasn't expecting this many horrible hacks around readout. So basiclly it looks like we have very little in terms of useful readout+state checking. > + if (crtc_state->bigjoiner && crtc_state->hw.active) { > + struct intel_crtc *slave = crtc_state->bigjoiner_linked_crtc; > + struct intel_crtc_state *slave_crtc_state = > + to_intel_crtc_state(slave->base.state); > + > + copy_bigjoiner_crtc_state(slave_crtc_state, crtc_state); > + slave->base.mode = crtc->base.mode; > + > + cdclk_state->min_cdclk[slave->pipe] = min_cdclk; > + cdclk_state->min_voltage_level[slave->pipe] = > + crtc_state->min_voltage_level; > + > + for_each_intel_plane_on_crtc(&dev_priv->drm, slave, plane) { > + const struct intel_plane_state *plane_state = > + to_intel_plane_state(plane->base.state); > + > + /* > + * FIXME don't have the fb yet, so can't > + * use intel_plane_data_rate() :( > + */ > + if (plane_state->uapi.visible) > + crtc_state->data_rate[plane->id] = > + 4 * crtc_state->pixel_rate; > + else > + crtc_state->data_rate[plane->id] = 0; > + } > + > + intel_bw_crtc_update(bw_state, slave_crtc_state); > + drm_calc_timestamping_constants(&slave->base, > + &slave_crtc_state->hw.adjusted_mode); > + } > } > } > > -- > 2.19.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Mon, Oct 19, 2020 at 07:36:59PM +0300, Ville Syrjälä wrote: > On Thu, Oct 08, 2020 at 02:45:31PM -0700, Manasi Navare wrote: > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > Unfortunately I have no way to test this, but it should be correct > > if the bios sets up bigjoiner in a sane way. > > > > Skip iterating over bigjoiner slaves, only the master has the state we > > care about. > > > > Add the width of the bigjoiner slave to the reconstructed fb. > > > > Hide the bigjoiner slave to userspace, and double the mode on bigjoiner > > master. > > > > And last, disable bigjoiner slave from primary if reconstruction fails. > > > > v2: > > * Manual Rebase (Manasi) > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display.c | 64 +++++++++++++++++++- > > 1 file changed, 62 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > index faf8bf757bed..aa981aa4f6a1 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > @@ -3619,6 +3619,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > struct intel_plane *intel_plane = to_intel_plane(primary); > > struct intel_plane_state *intel_state = > > to_intel_plane_state(plane_state); > > + struct intel_crtc_state *crtc_state = > > Whitespace fail. Didn't checkpatch complain? I will fix this > > > + to_intel_crtc_state(intel_crtc->base.state); > > struct drm_framebuffer *fb; > > struct i915_vma *vma; > > > > @@ -3641,7 +3643,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > if (c == &intel_crtc->base) > > continue; > > > > - if (!to_intel_crtc(c)->active) > > + if (!to_intel_crtc_state(c->state)->uapi.active) > > continue; > > > > state = to_intel_plane_state(c->primary->state); > > @@ -3663,6 +3665,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > * pretend the BIOS never had it enabled. > > */ > > intel_plane_disable_noatomic(intel_crtc, intel_plane); > > + if (crtc_state->bigjoiner) { > > + struct intel_crtc *slave = > > + crtc_state->bigjoiner_linked_crtc; > > + intel_plane_disable_noatomic(slave, to_intel_plane(slave->base.primary)); > > + } > > > > return; > > > > @@ -10687,6 +10694,7 @@ static void > > skl_get_initial_plane_config(struct intel_crtc *crtc, > > struct intel_initial_plane_config *plane_config) > > { > > + struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); > > struct drm_device *dev = crtc->base.dev; > > struct drm_i915_private *dev_priv = to_i915(dev); > > struct intel_plane *plane = to_intel_plane(crtc->base.primary); > > @@ -10795,6 +10803,18 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > > fb->height = ((val >> 16) & 0xffff) + 1; > > fb->width = ((val >> 0) & 0xffff) + 1; > > > > + /* add bigjoiner slave as well, if the fb stretches both */ > > + if (crtc_state->bigjoiner) { > > + enum pipe bigjoiner_pipe = crtc_state->bigjoiner_linked_crtc->pipe; > > + > > + if (fb->width == crtc_state->pipe_src_w && > > + (intel_de_read(dev_priv, PLANE_SURF(pipe, plane_id)) & 0xfffff000) == plane_config->base) { > > + val = intel_de_read(dev_priv, PLANE_SIZE(bigjoiner_pipe, plane_id)); > > + fb->height += ((val >> 16) & 0xfff) + 1; > > + fb->width += ((val >> 0) & 0x1fff) + 1; > > This looks wrong. Why is it wrong? Double checked the plane size width and height bits from bspec and the mask looks correct here. Can you elaborate on what is wrong here? > > > + } > > + } > > + > > val = intel_de_read(dev_priv, PLANE_STRIDE(pipe, plane_id)); > > stride_mult = skl_plane_stride_mult(fb, 0, DRM_MODE_ROTATE_0); > > fb->pitches[0] = (val & 0x3ff) * stride_mult; > > @@ -18793,7 +18813,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, > > > > /* Adjust the state of the output pipe according to whether we > > * have active connectors/encoders. */ > > - if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc)) > > + if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) && > > + !crtc_state->bigjoiner_slave) > > intel_crtc_disable_noatomic(crtc, ctx); > > > > if (crtc_state->hw.active || HAS_GMCH(dev_priv)) { > > @@ -19072,6 +19093,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > > struct intel_plane *plane; > > int min_cdclk = 0; > > > > + if (crtc_state->bigjoiner_slave) > > + continue; > > + > > if (crtc_state->hw.active) { > > struct drm_display_mode mode; > > > > @@ -19096,6 +19120,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > > mode.hdisplay = crtc_state->pipe_src_w; > > mode.vdisplay = crtc_state->pipe_src_h; > > > > + if (crtc_state->bigjoiner) > > + mode.hdisplay *= 2; > > + > > intel_crtc_compute_pixel_rate(crtc_state); > > > > intel_crtc_update_active_timings(crtc_state); > > @@ -19146,6 +19173,39 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) > > intel_bw_crtc_update(bw_state, crtc_state); > > > > intel_pipe_config_sanity_check(dev_priv, crtc_state); > > + > > + /* discard our incomplete slave state, copy it from master */ > > Wasn't expecting this many horrible hacks around readout. So basiclly it > looks like we have very little in terms of useful readout+state > checking. Yes we can fine tune this afterw e have basic functionality upstreamed. Manasi > > > + if (crtc_state->bigjoiner && crtc_state->hw.active) { > > + struct intel_crtc *slave = crtc_state->bigjoiner_linked_crtc; > > + struct intel_crtc_state *slave_crtc_state = > > + to_intel_crtc_state(slave->base.state); > > + > > + copy_bigjoiner_crtc_state(slave_crtc_state, crtc_state); > > + slave->base.mode = crtc->base.mode; > > + > > + cdclk_state->min_cdclk[slave->pipe] = min_cdclk; > > + cdclk_state->min_voltage_level[slave->pipe] = > > + crtc_state->min_voltage_level; > > + > > + for_each_intel_plane_on_crtc(&dev_priv->drm, slave, plane) { > > + const struct intel_plane_state *plane_state = > > + to_intel_plane_state(plane->base.state); > > + > > + /* > > + * FIXME don't have the fb yet, so can't > > + * use intel_plane_data_rate() :( > > + */ > > + if (plane_state->uapi.visible) > > + crtc_state->data_rate[plane->id] = > > + 4 * crtc_state->pixel_rate; > > + else > > + crtc_state->data_rate[plane->id] = 0; > > + } > > + > > + intel_bw_crtc_update(bw_state, slave_crtc_state); > > + drm_calc_timestamping_constants(&slave->base, > > + &slave_crtc_state->hw.adjusted_mode); > > + } > > } > > } > > > > -- > > 2.19.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
On Mon, Oct 19, 2020 at 03:45:04PM -0700, Navare, Manasi wrote: > On Mon, Oct 19, 2020 at 07:36:59PM +0300, Ville Syrjälä wrote: > > On Thu, Oct 08, 2020 at 02:45:31PM -0700, Manasi Navare wrote: > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > Unfortunately I have no way to test this, but it should be correct > > > if the bios sets up bigjoiner in a sane way. > > > > > > Skip iterating over bigjoiner slaves, only the master has the state we > > > care about. > > > > > > Add the width of the bigjoiner slave to the reconstructed fb. > > > > > > Hide the bigjoiner slave to userspace, and double the mode on bigjoiner > > > master. > > > > > > And last, disable bigjoiner slave from primary if reconstruction fails. > > > > > > v2: > > > * Manual Rebase (Manasi) > > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > --- > > > drivers/gpu/drm/i915/display/intel_display.c | 64 +++++++++++++++++++- > > > 1 file changed, 62 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > index faf8bf757bed..aa981aa4f6a1 100644 > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > @@ -3619,6 +3619,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > struct intel_plane *intel_plane = to_intel_plane(primary); > > > struct intel_plane_state *intel_state = > > > to_intel_plane_state(plane_state); > > > + struct intel_crtc_state *crtc_state = > > > > Whitespace fail. Didn't checkpatch complain? > > I will fix this > > > > > > + to_intel_crtc_state(intel_crtc->base.state); > > > struct drm_framebuffer *fb; > > > struct i915_vma *vma; > > > > > > @@ -3641,7 +3643,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > if (c == &intel_crtc->base) > > > continue; > > > > > > - if (!to_intel_crtc(c)->active) > > > + if (!to_intel_crtc_state(c->state)->uapi.active) > > > continue; > > > > > > state = to_intel_plane_state(c->primary->state); > > > @@ -3663,6 +3665,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > * pretend the BIOS never had it enabled. > > > */ > > > intel_plane_disable_noatomic(intel_crtc, intel_plane); > > > + if (crtc_state->bigjoiner) { > > > + struct intel_crtc *slave = > > > + crtc_state->bigjoiner_linked_crtc; > > > + intel_plane_disable_noatomic(slave, to_intel_plane(slave->base.primary)); > > > + } > > > > > > return; > > > > > > @@ -10687,6 +10694,7 @@ static void > > > skl_get_initial_plane_config(struct intel_crtc *crtc, > > > struct intel_initial_plane_config *plane_config) > > > { > > > + struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); > > > struct drm_device *dev = crtc->base.dev; > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > struct intel_plane *plane = to_intel_plane(crtc->base.primary); > > > @@ -10795,6 +10803,18 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > > > fb->height = ((val >> 16) & 0xffff) + 1; > > > fb->width = ((val >> 0) & 0xffff) + 1; > > > > > > + /* add bigjoiner slave as well, if the fb stretches both */ > > > + if (crtc_state->bigjoiner) { > > > + enum pipe bigjoiner_pipe = crtc_state->bigjoiner_linked_crtc->pipe; > > > + > > > + if (fb->width == crtc_state->pipe_src_w && > > > + (intel_de_read(dev_priv, PLANE_SURF(pipe, plane_id)) & 0xfffff000) == plane_config->base) { > > > + val = intel_de_read(dev_priv, PLANE_SIZE(bigjoiner_pipe, plane_id)); > > > + fb->height += ((val >> 16) & 0xfff) + 1; > > > + fb->width += ((val >> 0) & 0x1fff) + 1; > > > > This looks wrong. > > Why is it wrong? Double checked the plane size width and height bits from bspec > and the mask looks correct here. > Can you elaborate on what is wrong here? The pipes are side-by-side.
On Tue, Oct 20, 2020 at 09:45:02PM +0300, Ville Syrjälä wrote: > On Mon, Oct 19, 2020 at 03:45:04PM -0700, Navare, Manasi wrote: > > On Mon, Oct 19, 2020 at 07:36:59PM +0300, Ville Syrjälä wrote: > > > On Thu, Oct 08, 2020 at 02:45:31PM -0700, Manasi Navare wrote: > > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > Unfortunately I have no way to test this, but it should be correct > > > > if the bios sets up bigjoiner in a sane way. > > > > > > > > Skip iterating over bigjoiner slaves, only the master has the state we > > > > care about. > > > > > > > > Add the width of the bigjoiner slave to the reconstructed fb. > > > > > > > > Hide the bigjoiner slave to userspace, and double the mode on bigjoiner > > > > master. > > > > > > > > And last, disable bigjoiner slave from primary if reconstruction fails. > > > > > > > > v2: > > > > * Manual Rebase (Manasi) > > > > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > --- > > > > drivers/gpu/drm/i915/display/intel_display.c | 64 +++++++++++++++++++- > > > > 1 file changed, 62 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > index faf8bf757bed..aa981aa4f6a1 100644 > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > @@ -3619,6 +3619,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > > struct intel_plane *intel_plane = to_intel_plane(primary); > > > > struct intel_plane_state *intel_state = > > > > to_intel_plane_state(plane_state); > > > > + struct intel_crtc_state *crtc_state = > > > > > > Whitespace fail. Didn't checkpatch complain? > > > > I will fix this > > > > > > > > > + to_intel_crtc_state(intel_crtc->base.state); > > > > struct drm_framebuffer *fb; > > > > struct i915_vma *vma; > > > > > > > > @@ -3641,7 +3643,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > > if (c == &intel_crtc->base) > > > > continue; > > > > > > > > - if (!to_intel_crtc(c)->active) > > > > + if (!to_intel_crtc_state(c->state)->uapi.active) > > > > continue; > > > > > > > > state = to_intel_plane_state(c->primary->state); > > > > @@ -3663,6 +3665,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > > * pretend the BIOS never had it enabled. > > > > */ > > > > intel_plane_disable_noatomic(intel_crtc, intel_plane); > > > > + if (crtc_state->bigjoiner) { > > > > + struct intel_crtc *slave = > > > > + crtc_state->bigjoiner_linked_crtc; > > > > + intel_plane_disable_noatomic(slave, to_intel_plane(slave->base.primary)); > > > > + } > > > > > > > > return; > > > > > > > > @@ -10687,6 +10694,7 @@ static void > > > > skl_get_initial_plane_config(struct intel_crtc *crtc, > > > > struct intel_initial_plane_config *plane_config) > > > > { > > > > + struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); > > > > struct drm_device *dev = crtc->base.dev; > > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > struct intel_plane *plane = to_intel_plane(crtc->base.primary); > > > > @@ -10795,6 +10803,18 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > > > > fb->height = ((val >> 16) & 0xffff) + 1; > > > > fb->width = ((val >> 0) & 0xffff) + 1; > > > > > > > > + /* add bigjoiner slave as well, if the fb stretches both */ > > > > + if (crtc_state->bigjoiner) { > > > > + enum pipe bigjoiner_pipe = crtc_state->bigjoiner_linked_crtc->pipe; > > > > + > > > > + if (fb->width == crtc_state->pipe_src_w && > > > > + (intel_de_read(dev_priv, PLANE_SURF(pipe, plane_id)) & 0xfffff000) == plane_config->base) { > > > > + val = intel_de_read(dev_priv, PLANE_SIZE(bigjoiner_pipe, plane_id)); > > > > + fb->height += ((val >> 16) & 0xfff) + 1; > > > > + fb->width += ((val >> 0) & 0x1fff) + 1; > > > > > > This looks wrong. > > > > Why is it wrong? Double checked the plane size width and height bits from bspec > > and the mask looks correct here. > > Can you elaborate on what is wrong here? > > The pipes are side-by-side. Yes the master and slave pipes are consecutive, still not sure what is required to correct the logic here Manasi > > -- > Ville Syrjälä > Intel
On Tue, Oct 20, 2020 at 11:57:52AM -0700, Navare, Manasi wrote: > On Tue, Oct 20, 2020 at 09:45:02PM +0300, Ville Syrjälä wrote: > > On Mon, Oct 19, 2020 at 03:45:04PM -0700, Navare, Manasi wrote: > > > On Mon, Oct 19, 2020 at 07:36:59PM +0300, Ville Syrjälä wrote: > > > > On Thu, Oct 08, 2020 at 02:45:31PM -0700, Manasi Navare wrote: > > > > > From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > > > > > > Unfortunately I have no way to test this, but it should be correct > > > > > if the bios sets up bigjoiner in a sane way. > > > > > > > > > > Skip iterating over bigjoiner slaves, only the master has the state we > > > > > care about. > > > > > > > > > > Add the width of the bigjoiner slave to the reconstructed fb. > > > > > > > > > > Hide the bigjoiner slave to userspace, and double the mode on bigjoiner > > > > > master. > > > > > > > > > > And last, disable bigjoiner slave from primary if reconstruction fails. > > > > > > > > > > v2: > > > > > * Manual Rebase (Manasi) > > > > > > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com> > > > > > --- > > > > > drivers/gpu/drm/i915/display/intel_display.c | 64 +++++++++++++++++++- > > > > > 1 file changed, 62 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > > > > > index faf8bf757bed..aa981aa4f6a1 100644 > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c > > > > > @@ -3619,6 +3619,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > > > struct intel_plane *intel_plane = to_intel_plane(primary); > > > > > struct intel_plane_state *intel_state = > > > > > to_intel_plane_state(plane_state); > > > > > + struct intel_crtc_state *crtc_state = > > > > > > > > Whitespace fail. Didn't checkpatch complain? > > > > > > I will fix this > > > > > > > > > > > > + to_intel_crtc_state(intel_crtc->base.state); > > > > > struct drm_framebuffer *fb; > > > > > struct i915_vma *vma; > > > > > > > > > > @@ -3641,7 +3643,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > > > if (c == &intel_crtc->base) > > > > > continue; > > > > > > > > > > - if (!to_intel_crtc(c)->active) > > > > > + if (!to_intel_crtc_state(c->state)->uapi.active) > > > > > continue; > > > > > > > > > > state = to_intel_plane_state(c->primary->state); > > > > > @@ -3663,6 +3665,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, > > > > > * pretend the BIOS never had it enabled. > > > > > */ > > > > > intel_plane_disable_noatomic(intel_crtc, intel_plane); > > > > > + if (crtc_state->bigjoiner) { > > > > > + struct intel_crtc *slave = > > > > > + crtc_state->bigjoiner_linked_crtc; > > > > > + intel_plane_disable_noatomic(slave, to_intel_plane(slave->base.primary)); > > > > > + } > > > > > > > > > > return; > > > > > > > > > > @@ -10687,6 +10694,7 @@ static void > > > > > skl_get_initial_plane_config(struct intel_crtc *crtc, > > > > > struct intel_initial_plane_config *plane_config) > > > > > { > > > > > + struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); > > > > > struct drm_device *dev = crtc->base.dev; > > > > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > > struct intel_plane *plane = to_intel_plane(crtc->base.primary); > > > > > @@ -10795,6 +10803,18 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, > > > > > fb->height = ((val >> 16) & 0xffff) + 1; > > > > > fb->width = ((val >> 0) & 0xffff) + 1; > > > > > > > > > > + /* add bigjoiner slave as well, if the fb stretches both */ > > > > > + if (crtc_state->bigjoiner) { > > > > > + enum pipe bigjoiner_pipe = crtc_state->bigjoiner_linked_crtc->pipe; > > > > > + > > > > > + if (fb->width == crtc_state->pipe_src_w && > > > > > + (intel_de_read(dev_priv, PLANE_SURF(pipe, plane_id)) & 0xfffff000) == plane_config->base) { > > > > > + val = intel_de_read(dev_priv, PLANE_SIZE(bigjoiner_pipe, plane_id)); > > > > > + fb->height += ((val >> 16) & 0xfff) + 1; > > > > > + fb->width += ((val >> 0) & 0x1fff) + 1; > > > > > > > > This looks wrong. > > > > > > Why is it wrong? Double checked the plane size width and height bits from bspec > > > and the mask looks correct here. > > > Can you elaborate on what is wrong here? > > > > The pipes are side-by-side. Gotcha, so Only the fb->width needs to be modified to add the width of slave pipe and keep height as is Right? I will fix this in the next rev Manasi > > Yes the master and slave pipes are consecutive, still not sure what is required to correct the logic here > > Manasi > > > > > -- > > Ville Syrjälä > > Intel > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index faf8bf757bed..aa981aa4f6a1 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -3619,6 +3619,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, struct intel_plane *intel_plane = to_intel_plane(primary); struct intel_plane_state *intel_state = to_intel_plane_state(plane_state); + struct intel_crtc_state *crtc_state = + to_intel_crtc_state(intel_crtc->base.state); struct drm_framebuffer *fb; struct i915_vma *vma; @@ -3641,7 +3643,7 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, if (c == &intel_crtc->base) continue; - if (!to_intel_crtc(c)->active) + if (!to_intel_crtc_state(c->state)->uapi.active) continue; state = to_intel_plane_state(c->primary->state); @@ -3663,6 +3665,11 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc, * pretend the BIOS never had it enabled. */ intel_plane_disable_noatomic(intel_crtc, intel_plane); + if (crtc_state->bigjoiner) { + struct intel_crtc *slave = + crtc_state->bigjoiner_linked_crtc; + intel_plane_disable_noatomic(slave, to_intel_plane(slave->base.primary)); + } return; @@ -10687,6 +10694,7 @@ static void skl_get_initial_plane_config(struct intel_crtc *crtc, struct intel_initial_plane_config *plane_config) { + struct intel_crtc_state *crtc_state = to_intel_crtc_state(crtc->base.state); struct drm_device *dev = crtc->base.dev; struct drm_i915_private *dev_priv = to_i915(dev); struct intel_plane *plane = to_intel_plane(crtc->base.primary); @@ -10795,6 +10803,18 @@ skl_get_initial_plane_config(struct intel_crtc *crtc, fb->height = ((val >> 16) & 0xffff) + 1; fb->width = ((val >> 0) & 0xffff) + 1; + /* add bigjoiner slave as well, if the fb stretches both */ + if (crtc_state->bigjoiner) { + enum pipe bigjoiner_pipe = crtc_state->bigjoiner_linked_crtc->pipe; + + if (fb->width == crtc_state->pipe_src_w && + (intel_de_read(dev_priv, PLANE_SURF(pipe, plane_id)) & 0xfffff000) == plane_config->base) { + val = intel_de_read(dev_priv, PLANE_SIZE(bigjoiner_pipe, plane_id)); + fb->height += ((val >> 16) & 0xfff) + 1; + fb->width += ((val >> 0) & 0x1fff) + 1; + } + } + val = intel_de_read(dev_priv, PLANE_STRIDE(pipe, plane_id)); stride_mult = skl_plane_stride_mult(fb, 0, DRM_MODE_ROTATE_0); fb->pitches[0] = (val & 0x3ff) * stride_mult; @@ -18793,7 +18813,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc, /* Adjust the state of the output pipe according to whether we * have active connectors/encoders. */ - if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc)) + if (crtc_state->hw.active && !intel_crtc_has_encoders(crtc) && + !crtc_state->bigjoiner_slave) intel_crtc_disable_noatomic(crtc, ctx); if (crtc_state->hw.active || HAS_GMCH(dev_priv)) { @@ -19072,6 +19093,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) struct intel_plane *plane; int min_cdclk = 0; + if (crtc_state->bigjoiner_slave) + continue; + if (crtc_state->hw.active) { struct drm_display_mode mode; @@ -19096,6 +19120,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) mode.hdisplay = crtc_state->pipe_src_w; mode.vdisplay = crtc_state->pipe_src_h; + if (crtc_state->bigjoiner) + mode.hdisplay *= 2; + intel_crtc_compute_pixel_rate(crtc_state); intel_crtc_update_active_timings(crtc_state); @@ -19146,6 +19173,39 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev) intel_bw_crtc_update(bw_state, crtc_state); intel_pipe_config_sanity_check(dev_priv, crtc_state); + + /* discard our incomplete slave state, copy it from master */ + if (crtc_state->bigjoiner && crtc_state->hw.active) { + struct intel_crtc *slave = crtc_state->bigjoiner_linked_crtc; + struct intel_crtc_state *slave_crtc_state = + to_intel_crtc_state(slave->base.state); + + copy_bigjoiner_crtc_state(slave_crtc_state, crtc_state); + slave->base.mode = crtc->base.mode; + + cdclk_state->min_cdclk[slave->pipe] = min_cdclk; + cdclk_state->min_voltage_level[slave->pipe] = + crtc_state->min_voltage_level; + + for_each_intel_plane_on_crtc(&dev_priv->drm, slave, plane) { + const struct intel_plane_state *plane_state = + to_intel_plane_state(plane->base.state); + + /* + * FIXME don't have the fb yet, so can't + * use intel_plane_data_rate() :( + */ + if (plane_state->uapi.visible) + crtc_state->data_rate[plane->id] = + 4 * crtc_state->pixel_rate; + else + crtc_state->data_rate[plane->id] = 0; + } + + intel_bw_crtc_update(bw_state, slave_crtc_state); + drm_calc_timestamping_constants(&slave->base, + &slave_crtc_state->hw.adjusted_mode); + } } }