Re: [Intel-gfx] [PATCH v2 3/3] drm/i915: Add connector dbgfs for all connectors

[Jani Nikula <jani.nikula@intel.com>]

On Tue, 24 Mar 2020, Anshuman Gupta <anshuman.gupta@intel.com> wrote:
> Add connector debugfs attributes for each intel
> connector which is getting register.

Okay, so this is a good idea, and for that,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> v2:
> - adding connector debugfs for each connector in
>   intel_connector_register() to fix CI failure for legacy connectors.

However, it's *not* a good idea for the purpose of registering the lpsp
debugfs file for every connector out there. There's no point in that, at
all. You need to register the file only when it makes sense in
intel_connector_debugfs_add(), and not for every conceivable connector
out there.

And your igt test absolutely *must* check if the debugfs is there or
not, and handle it gracefully.

BR,
Jani.

>
> Signed-off-by: Anshuman Gupta <anshuman.gupta@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_connector.c | 3 +++
>  drivers/gpu/drm/i915/display/intel_dp.c        | 3 ---
>  drivers/gpu/drm/i915/display/intel_hdmi.c      | 3 ---
>  3 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_connector.c b/drivers/gpu/drm/i915/display/intel_connector.c
> index 903e49659f56..0cf5fe326a0b 100644
> --- a/drivers/gpu/drm/i915/display/intel_connector.c
> +++ b/drivers/gpu/drm/i915/display/intel_connector.c
> @@ -33,6 +33,7 @@
>  
>  #include "i915_drv.h"
>  #include "intel_connector.h"
> +#include "intel_display_debugfs.h"
>  #include "intel_display_types.h"
>  #include "intel_hdcp.h"
>  
> @@ -123,6 +124,8 @@ int intel_connector_register(struct drm_connector *connector)
>  		goto err_backlight;
>  	}
>  
> +	intel_connector_debugfs_add(connector);
> +
>  	return 0;
>  
>  err_backlight:
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7f1a4e55cda1..c4352d013c29 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -48,7 +48,6 @@
>  #include "intel_audio.h"
>  #include "intel_connector.h"
>  #include "intel_ddi.h"
> -#include "intel_display_debugfs.h"
>  #include "intel_display_types.h"
>  #include "intel_dp.h"
>  #include "intel_dp_link_training.h"
> @@ -6204,8 +6203,6 @@ intel_dp_connector_register(struct drm_connector *connector)
>  	if (ret)
>  		return ret;
>  
> -	intel_connector_debugfs_add(connector);
> -
>  	DRM_DEBUG_KMS("registering %s bus for %s\n",
>  		      intel_dp->aux.name, connector->kdev->kobj.name);
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 39930232b253..2d4dced7143e 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -44,7 +44,6 @@
>  #include "intel_audio.h"
>  #include "intel_connector.h"
>  #include "intel_ddi.h"
> -#include "intel_display_debugfs.h"
>  #include "intel_display_types.h"
>  #include "intel_dp.h"
>  #include "intel_dpio_phy.h"
> @@ -2813,8 +2812,6 @@ intel_hdmi_connector_register(struct drm_connector *connector)
>  	if (ret)
>  		return ret;
>  
> -	intel_connector_debugfs_add(connector);
> -
>  	intel_hdmi_create_i2c_symlink(connector);
>  
>  	return ret;

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx