| Message ID | 20240926072638.3689367-3-ankit.k.nautiyal@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Ultrajoiner basic functionality series | expand |
On Thu, Sep 26, 2024 at 12:56:25PM +0530, Ankit Nautiyal wrote: > Currently we support joiner only for DP encoder. > Do not create the debugfs for joiner if DP does not support the joiner. > This will also help avoiding cases where config has eDP MSO, with which > we do not support joiner. > > v2: Check for intel_dp_has_joiner and avoid creating debugfs if not > supported. (Ville) > v3: Remove HAS_BIGJOINER check. (Ville) > v4: Reverse checks for connector type and intel_dp_has_joiner(). (Ville) > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display_debugfs.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > index 890ef7067b77..08adeaa2e87f 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > @@ -1328,6 +1328,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector) > struct drm_i915_private *i915 = to_i915(connector->base.dev); > struct dentry *root = connector->base.debugfs_entry; > int connector_type = connector->base.connector_type; > + struct intel_dp *intel_dp = intel_attached_dp(connector); I'd probably drop the local variable entirely since it can give us garbage for non-dp stuff. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > /* The connector must have been registered beforehands. */ > if (!root) > @@ -1362,9 +1363,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector) > connector, &i915_dsc_fractional_bpp_fops); > } > > - if (HAS_BIGJOINER(i915) && > - (connector_type == DRM_MODE_CONNECTOR_DisplayPort || > - connector_type == DRM_MODE_CONNECTOR_eDP)) { > + if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort || > + connector_type == DRM_MODE_CONNECTOR_eDP) && > + intel_dp_has_joiner(intel_dp)) { > debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root, > &connector->force_bigjoiner_enable); > } > -- > 2.45.2
On 9/26/2024 4:44 PM, Ville Syrjälä wrote: > On Thu, Sep 26, 2024 at 12:56:25PM +0530, Ankit Nautiyal wrote: >> Currently we support joiner only for DP encoder. >> Do not create the debugfs for joiner if DP does not support the joiner. >> This will also help avoiding cases where config has eDP MSO, with which >> we do not support joiner. >> >> v2: Check for intel_dp_has_joiner and avoid creating debugfs if not >> supported. (Ville) >> v3: Remove HAS_BIGJOINER check. (Ville) >> v4: Reverse checks for connector type and intel_dp_has_joiner(). (Ville) >> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display_debugfs.c | 7 ++++--- >> 1 file changed, 4 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c >> index 890ef7067b77..08adeaa2e87f 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c >> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c >> @@ -1328,6 +1328,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector) >> struct drm_i915_private *i915 = to_i915(connector->base.dev); >> struct dentry *root = connector->base.debugfs_entry; >> int connector_type = connector->base.connector_type; >> + struct intel_dp *intel_dp = intel_attached_dp(connector); > I'd probably drop the local variable entirely since it > can give us garbage for non-dp stuff. Yeah, can directly use intel_attached_dp(connector) to avoid having intel_dp with some garbage values for non DP connectors. Thanks for the review. As an aside, now that the first 4 patches (that are dealing with the debugfs) are reviewed, can I send them as separate series and merge them? This will help get the IGT changes merge for debugfs changes. Thanks & Regards, Ankit > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> >> /* The connector must have been registered beforehands. */ >> if (!root) >> @@ -1362,9 +1363,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector) >> connector, &i915_dsc_fractional_bpp_fops); >> } >> >> - if (HAS_BIGJOINER(i915) && >> - (connector_type == DRM_MODE_CONNECTOR_DisplayPort || >> - connector_type == DRM_MODE_CONNECTOR_eDP)) { >> + if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort || >> + connector_type == DRM_MODE_CONNECTOR_eDP) && >> + intel_dp_has_joiner(intel_dp)) { >> debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root, >> &connector->force_bigjoiner_enable); >> } >> -- >> 2.45.2
On Thu, Sep 26, 2024 at 06:37:46PM +0530, Nautiyal, Ankit K wrote: > > On 9/26/2024 4:44 PM, Ville Syrjälä wrote: > > On Thu, Sep 26, 2024 at 12:56:25PM +0530, Ankit Nautiyal wrote: > >> Currently we support joiner only for DP encoder. > >> Do not create the debugfs for joiner if DP does not support the joiner. > >> This will also help avoiding cases where config has eDP MSO, with which > >> we do not support joiner. > >> > >> v2: Check for intel_dp_has_joiner and avoid creating debugfs if not > >> supported. (Ville) > >> v3: Remove HAS_BIGJOINER check. (Ville) > >> v4: Reverse checks for connector type and intel_dp_has_joiner(). (Ville) > >> > >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_display_debugfs.c | 7 ++++--- > >> 1 file changed, 4 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > >> index 890ef7067b77..08adeaa2e87f 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c > >> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c > >> @@ -1328,6 +1328,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector) > >> struct drm_i915_private *i915 = to_i915(connector->base.dev); > >> struct dentry *root = connector->base.debugfs_entry; > >> int connector_type = connector->base.connector_type; > >> + struct intel_dp *intel_dp = intel_attached_dp(connector); > > I'd probably drop the local variable entirely since it > > can give us garbage for non-dp stuff. > > Yeah, can directly use intel_attached_dp(connector) to avoid having > intel_dp with some garbage values for non DP connectors. > > Thanks for the review. > > As an aside, now that the first 4 patches (that are dealing with the > debugfs) are reviewed, can I send them as separate series and merge them? > > This will help get the IGT changes merge for debugfs changes. Sure. Though CI is bogged down still, so dunno when you can expect results form there. > > Thanks & Regards, > > Ankit > > > > > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > >> > >> /* The connector must have been registered beforehands. */ > >> if (!root) > >> @@ -1362,9 +1363,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector) > >> connector, &i915_dsc_fractional_bpp_fops); > >> } > >> > >> - if (HAS_BIGJOINER(i915) && > >> - (connector_type == DRM_MODE_CONNECTOR_DisplayPort || > >> - connector_type == DRM_MODE_CONNECTOR_eDP)) { > >> + if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort || > >> + connector_type == DRM_MODE_CONNECTOR_eDP) && > >> + intel_dp_has_joiner(intel_dp)) { > >> debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root, > >> &connector->force_bigjoiner_enable); > >> } > >> -- > >> 2.45.2
diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c index 890ef7067b77..08adeaa2e87f 100644 --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c @@ -1328,6 +1328,7 @@ void intel_connector_debugfs_add(struct intel_connector *connector) struct drm_i915_private *i915 = to_i915(connector->base.dev); struct dentry *root = connector->base.debugfs_entry; int connector_type = connector->base.connector_type; + struct intel_dp *intel_dp = intel_attached_dp(connector); /* The connector must have been registered beforehands. */ if (!root) @@ -1362,9 +1363,9 @@ void intel_connector_debugfs_add(struct intel_connector *connector) connector, &i915_dsc_fractional_bpp_fops); } - if (HAS_BIGJOINER(i915) && - (connector_type == DRM_MODE_CONNECTOR_DisplayPort || - connector_type == DRM_MODE_CONNECTOR_eDP)) { + if ((connector_type == DRM_MODE_CONNECTOR_DisplayPort || + connector_type == DRM_MODE_CONNECTOR_eDP) && + intel_dp_has_joiner(intel_dp)) { debugfs_create_bool("i915_bigjoiner_force_enable", 0644, root, &connector->force_bigjoiner_enable); }
Currently we support joiner only for DP encoder. Do not create the debugfs for joiner if DP does not support the joiner. This will also help avoiding cases where config has eDP MSO, with which we do not support joiner. v2: Check for intel_dp_has_joiner and avoid creating debugfs if not supported. (Ville) v3: Remove HAS_BIGJOINER check. (Ville) v4: Reverse checks for connector type and intel_dp_has_joiner(). (Ville) Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> --- drivers/gpu/drm/i915/display/intel_display_debugfs.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)