On Mon, Oct 19, 2020 at 07:30:07PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 15, 2020 at 09:26:45AM -0700, Navare, Manasi wrote:
> > On Thu, Oct 15, 2020 at 02:52:47PM +0300, Ville Syrjälä wrote:
> > > On Wed, Oct 14, 2020 at 12:04:10PM -0700, Navare, Manasi wrote:
> > > > On Wed, Oct 14, 2020 at 02:26:34PM +0300, Ville Syrjälä wrote:
> > > > > On Thu, Oct 08, 2020 at 02:45:28PM -0700, Manasi Navare wrote:
> > > > > > From: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > > > > >
> > > > > > Small changes to intel_dp_mode_valid(), allow listing modes that
> > > > > > can only be supported in the bigjoiner configuration, which is
> > > > > > not supported yet.
> > > > > >
> > > > > > eDP does not support bigjoiner, so do not expose bigjoiner only
> > > > > > modes on the eDP port.
> > > > > >
> > > > > > v7:
> > > > > > * Add can_bigjoiner() helper (Ville)
> > > > > > * Pass bigjoiner to plane_size validation (Ville)
> > > > > > v6:
> > > > > > * Rebase after dp_downstream mode valid changes (Manasi)
> > > > > > v5:
> > > > > > * Increase max plane width to support 8K with bigjoiner (Maarten)
> > > > > > v4:
> > > > > > * Rebase (Manasi)
> > > > > >
> > > > > > Changes since v1:
> > > > > > - Disallow bigjoiner on eDP.
> > > > > > Changes since v2:
> > > > > > - Rename intel_dp_downstream_max_dotclock to intel_dp_max_dotclock,
> > > > > > and split off the downstream and source checking to its own function.
> > > > > > (Ville)
> > > > > > v3:
> > > > > > * Rebase (Manasi)
> > > > > >
> > > > > > Signed-off-by: Manasi Navare <manasi.d.navare@xxxxxxxxx>
> > > > > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/display/intel_display.c | 5 +-
> > > > > > drivers/gpu/drm/i915/display/intel_display.h | 3 +-
> > > > > > drivers/gpu/drm/i915/display/intel_dp.c | 126 +++++++++++++++----
> > > > > > drivers/gpu/drm/i915/display/intel_dp_mst.c | 2 +-
> > > > > > drivers/gpu/drm/i915/display/intel_dsi.c | 2 +-
> > > > > > drivers/gpu/drm/i915/display/intel_hdmi.c | 2 +-
> > > > > > 6 files changed, 111 insertions(+), 29 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > index 723766b1eae3..cc540c7b7dcd 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > @@ -17642,7 +17642,8 @@ intel_mode_valid(struct drm_device *dev,
> > > > > >
> > > > > > enum drm_mode_status
> > > > > > intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > > > - const struct drm_display_mode *mode)
> > > > > > + const struct drm_display_mode *mode,
> > > > > > + bool bigjoiner)
> > > > > > {
> > > > > > int plane_width_max, plane_height_max;
> > > > > >
> > > > > > @@ -17659,7 +17660,7 @@ intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > > > * too big for that.
> > > > > > */
> > > > > > if (INTEL_GEN(dev_priv) >= 11) {
> > > > > > - plane_width_max = 5120;
> > > > > > + plane_width_max = 5120 << bigjoiner;
> > > > > > plane_height_max = 4320;
> > > > > > } else {
> > > > > > plane_width_max = 5120;
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
> > > > > > index d10b7c8cde3f..3d860a9da8fe 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.h
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.h
> > > > > > @@ -496,7 +496,8 @@ u32 intel_plane_fb_max_stride(struct drm_i915_private *dev_priv,
> > > > > > bool intel_plane_can_remap(const struct intel_plane_state *plane_state);
> > > > > > enum drm_mode_status
> > > > > > intel_mode_valid_max_plane_size(struct drm_i915_private *dev_priv,
> > > > > > - const struct drm_display_mode *mode);
> > > > > > + const struct drm_display_mode *mode,
> > > > > > + bool bigjoiner);
> > > > > > enum phy intel_port_to_phy(struct drm_i915_private *i915, enum port port);
> > > > > > bool is_trans_port_sync_mode(const struct intel_crtc_state *state);
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > index 8a522edd7386..af2ff425e5d5 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> > > > > > @@ -247,6 +247,29 @@ intel_dp_max_data_rate(int max_link_clock, int max_lanes)
> > > > > > return max_link_clock * max_lanes;
> > > > > > }
> > > > > >
> > > > > > +static int source_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > > > > +{
> > > > > > + struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > > > > > + struct intel_encoder *encoder = &intel_dig_port->base;
> > > > > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > > > > > +
> > > > > > + if (allow_bigjoiner && INTEL_GEN(dev_priv) >= 11 && !intel_dp_is_edp(intel_dp))
> > > > > > + return 2 * dev_priv->max_dotclk_freq;
> > > > > > +
> > > > > > + return dev_priv->max_dotclk_freq;
> > > > > > +}
> > > > > > +
> > > > > > +static int
> > > > > > +intel_dp_max_dotclock(struct intel_dp *intel_dp, bool allow_bigjoiner)
> > > > > > +{
> > > > > > + int max_dotclk = source_max_dotclock(intel_dp, allow_bigjoiner);
> > > > > > +
> > > > > > + if (intel_dp->dfp.max_dotclock)
> > > > >
> > > > > No. dfp checks should stay where they are.
> > > >
> > > > I am using dfp.max_dotclock because we populate that with drm_dp_downstream_max_dotclock()
> > > > should that be used here directly from drm_dp_downstream_max_dotclock instead of using dfp.maxdotclock ?
> > >
> > > Can you explain how bigjoiner and DFP dotclock limits relate to each
> > > other?
> >
> > Before the dfp dotclock checks were added, we were obtaining the max dotclock as min (source_max_dotclock, downstream max dotclock from dpcd)
> > And thats why I was using the dfp.max_dotclock
> >
> > But while addressing your feedback , I have now max_dotclock = source max dotclock
> > and the downstream max dotclock checks happen in intel_dp_mode_valid_downstream(), so I think we dont need to consider
> > this here in max dotclock computation.
> >
> > Is this correct?
>
> DFP's have nothing to do with bigjoiner. It is a purely internal
> implemntation detail of our display engine. So mixing up the two
> would be very wrong.
I understand that bigjoiner is a purely internal implementation
detail of our display engine, however whethera mode can be supported with bigjoiner
also depends on whether the clock of that mode is supported
by downstream port hence we were using the max downstream dotclock.
But I have removed the downstream port clock check for now and only using
the source dotclock check tod ecide on whether the mode is valid with bigjoiner or not since
if the downstream port doesnt support it, it will get pruned in the newly added
intel_dp_downstream_mode_valid() function.
Manasi
>
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
