diff mbox series

drm/i915/bios: debug log ddi port info after parsing

Message ID 20220621123732.1118437-1-jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/bios: debug log ddi port info after parsing | expand

Commit Message

Jani Nikula June 21, 2022, 12:37 p.m. UTC
The ddc pin and aux channel sanitization may disable DVI/HDMI and DP,
respectively, of ports parsed earlier, in "last one wins" fashion. With
parsing and printing interleaved, we'll end up logging support first and
disabling later anyway.

Now that we've split ddi port info parsing and printing, take it further
by doing the printing in a separate loop, fixing the logging.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bios.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Ville Syrjala June 23, 2022, 9:25 a.m. UTC | #1
On Tue, Jun 21, 2022 at 03:37:32PM +0300, Jani Nikula wrote:
> The ddc pin and aux channel sanitization may disable DVI/HDMI and DP,
> respectively, of ports parsed earlier, in "last one wins" fashion. With
> parsing and printing interleaved, we'll end up logging support first and
> disabling later anyway.
> 
> Now that we've split ddi port info parsing and printing, take it further
> by doing the printing in a separate loop, fixing the logging.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_bios.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> index ab23324c0402..51dde5bfd956 100644
> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> @@ -2670,8 +2670,6 @@ static void parse_ddi_port(struct intel_bios_encoder_data *devdata)
>  
>  	sanitize_device_type(devdata, port);
>  
> -	print_ddi_port(devdata, port);
> -
>  	if (intel_bios_encoder_supports_dvi(devdata))
>  		sanitize_ddc_pin(devdata, port);
>  
> @@ -2689,12 +2687,18 @@ static bool has_ddi_port_info(struct drm_i915_private *i915)
>  static void parse_ddi_ports(struct drm_i915_private *i915)
>  {
>  	struct intel_bios_encoder_data *devdata;
> +	enum port port;
>  
>  	if (!has_ddi_port_info(i915))
>  		return;
>  
>  	list_for_each_entry(devdata, &i915->vbt.display_devices, node)
>  		parse_ddi_port(devdata);
> +
> +	for_each_port(port) {
> +		if (i915->vbt.ports[port])
> +			print_ddi_port(i915->vbt.ports[port], port);
> +	}
>  }
>  
>  static void
> -- 
> 2.30.2
Jani Nikula June 23, 2022, 5:04 p.m. UTC | #2
On Thu, 23 Jun 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Jun 21, 2022 at 03:37:32PM +0300, Jani Nikula wrote:
>> The ddc pin and aux channel sanitization may disable DVI/HDMI and DP,
>> respectively, of ports parsed earlier, in "last one wins" fashion. With
>> parsing and printing interleaved, we'll end up logging support first and
>> disabling later anyway.
>> 
>> Now that we've split ddi port info parsing and printing, take it further
>> by doing the printing in a separate loop, fixing the logging.
>> 
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just realized this also changes the printing order from VBT child device
order to port number order. Is that a bug or a feature?

BR,
Jani.

>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bios.c | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> index ab23324c0402..51dde5bfd956 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> @@ -2670,8 +2670,6 @@ static void parse_ddi_port(struct intel_bios_encoder_data *devdata)
>>  
>>  	sanitize_device_type(devdata, port);
>>  
>> -	print_ddi_port(devdata, port);
>> -
>>  	if (intel_bios_encoder_supports_dvi(devdata))
>>  		sanitize_ddc_pin(devdata, port);
>>  
>> @@ -2689,12 +2687,18 @@ static bool has_ddi_port_info(struct drm_i915_private *i915)
>>  static void parse_ddi_ports(struct drm_i915_private *i915)
>>  {
>>  	struct intel_bios_encoder_data *devdata;
>> +	enum port port;
>>  
>>  	if (!has_ddi_port_info(i915))
>>  		return;
>>  
>>  	list_for_each_entry(devdata, &i915->vbt.display_devices, node)
>>  		parse_ddi_port(devdata);
>> +
>> +	for_each_port(port) {
>> +		if (i915->vbt.ports[port])
>> +			print_ddi_port(i915->vbt.ports[port], port);
>> +	}
>>  }
>>  
>>  static void
>> -- 
>> 2.30.2
Ville Syrjala June 23, 2022, 7:51 p.m. UTC | #3
On Thu, Jun 23, 2022 at 08:04:54PM +0300, Jani Nikula wrote:
> On Thu, 23 Jun 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > On Tue, Jun 21, 2022 at 03:37:32PM +0300, Jani Nikula wrote:
> >> The ddc pin and aux channel sanitization may disable DVI/HDMI and DP,
> >> respectively, of ports parsed earlier, in "last one wins" fashion. With
> >> parsing and printing interleaved, we'll end up logging support first and
> >> disabling later anyway.
> >> 
> >> Now that we've split ddi port info parsing and printing, take it further
> >> by doing the printing in a separate loop, fixing the logging.
> >> 
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >
> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Just realized this also changes the printing order from VBT child device
> order to port number order. Is that a bug or a feature?

I suppose it doesn't really matter. The only things where the order
really matters are conflicts and we should still get some extra debug
spew for those right?

> 
> BR,
> Jani.
> 
> >
> >> ---
> >>  drivers/gpu/drm/i915/display/intel_bios.c | 8 ++++++--
> >>  1 file changed, 6 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
> >> index ab23324c0402..51dde5bfd956 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
> >> @@ -2670,8 +2670,6 @@ static void parse_ddi_port(struct intel_bios_encoder_data *devdata)
> >>  
> >>  	sanitize_device_type(devdata, port);
> >>  
> >> -	print_ddi_port(devdata, port);
> >> -
> >>  	if (intel_bios_encoder_supports_dvi(devdata))
> >>  		sanitize_ddc_pin(devdata, port);
> >>  
> >> @@ -2689,12 +2687,18 @@ static bool has_ddi_port_info(struct drm_i915_private *i915)
> >>  static void parse_ddi_ports(struct drm_i915_private *i915)
> >>  {
> >>  	struct intel_bios_encoder_data *devdata;
> >> +	enum port port;
> >>  
> >>  	if (!has_ddi_port_info(i915))
> >>  		return;
> >>  
> >>  	list_for_each_entry(devdata, &i915->vbt.display_devices, node)
> >>  		parse_ddi_port(devdata);
> >> +
> >> +	for_each_port(port) {
> >> +		if (i915->vbt.ports[port])
> >> +			print_ddi_port(i915->vbt.ports[port], port);
> >> +	}
> >>  }
> >>  
> >>  static void
> >> -- 
> >> 2.30.2
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
Jani Nikula June 30, 2022, 12:06 p.m. UTC | #4
On Thu, 23 Jun 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Jun 23, 2022 at 08:04:54PM +0300, Jani Nikula wrote:
>> On Thu, 23 Jun 2022, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> > On Tue, Jun 21, 2022 at 03:37:32PM +0300, Jani Nikula wrote:
>> >> The ddc pin and aux channel sanitization may disable DVI/HDMI and DP,
>> >> respectively, of ports parsed earlier, in "last one wins" fashion. With
>> >> parsing and printing interleaved, we'll end up logging support first and
>> >> disabling later anyway.
>> >> 
>> >> Now that we've split ddi port info parsing and printing, take it further
>> >> by doing the printing in a separate loop, fixing the logging.
>> >> 
>> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >
>> > Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> 
>> Just realized this also changes the printing order from VBT child device
>> order to port number order. Is that a bug or a feature?
>
> I suppose it doesn't really matter. The only things where the order
> really matters are conflicts and we should still get some extra debug
> spew for those right?

Correct.

Pushed to din, thanks for the review.

BR,
Jani.



>
>> 
>> BR,
>> Jani.
>> 
>> >
>> >> ---
>> >>  drivers/gpu/drm/i915/display/intel_bios.c | 8 ++++++--
>> >>  1 file changed, 6 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
>> >> index ab23324c0402..51dde5bfd956 100644
>> >> --- a/drivers/gpu/drm/i915/display/intel_bios.c
>> >> +++ b/drivers/gpu/drm/i915/display/intel_bios.c
>> >> @@ -2670,8 +2670,6 @@ static void parse_ddi_port(struct intel_bios_encoder_data *devdata)
>> >>  
>> >>  	sanitize_device_type(devdata, port);
>> >>  
>> >> -	print_ddi_port(devdata, port);
>> >> -
>> >>  	if (intel_bios_encoder_supports_dvi(devdata))
>> >>  		sanitize_ddc_pin(devdata, port);
>> >>  
>> >> @@ -2689,12 +2687,18 @@ static bool has_ddi_port_info(struct drm_i915_private *i915)
>> >>  static void parse_ddi_ports(struct drm_i915_private *i915)
>> >>  {
>> >>  	struct intel_bios_encoder_data *devdata;
>> >> +	enum port port;
>> >>  
>> >>  	if (!has_ddi_port_info(i915))
>> >>  		return;
>> >>  
>> >>  	list_for_each_entry(devdata, &i915->vbt.display_devices, node)
>> >>  		parse_ddi_port(devdata);
>> >> +
>> >> +	for_each_port(port) {
>> >> +		if (i915->vbt.ports[port])
>> >> +			print_ddi_port(i915->vbt.ports[port], port);
>> >> +	}
>> >>  }
>> >>  
>> >>  static void
>> >> -- 
>> >> 2.30.2
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_bios.c b/drivers/gpu/drm/i915/display/intel_bios.c
index ab23324c0402..51dde5bfd956 100644
--- a/drivers/gpu/drm/i915/display/intel_bios.c
+++ b/drivers/gpu/drm/i915/display/intel_bios.c
@@ -2670,8 +2670,6 @@  static void parse_ddi_port(struct intel_bios_encoder_data *devdata)
 
 	sanitize_device_type(devdata, port);
 
-	print_ddi_port(devdata, port);
-
 	if (intel_bios_encoder_supports_dvi(devdata))
 		sanitize_ddc_pin(devdata, port);
 
@@ -2689,12 +2687,18 @@  static bool has_ddi_port_info(struct drm_i915_private *i915)
 static void parse_ddi_ports(struct drm_i915_private *i915)
 {
 	struct intel_bios_encoder_data *devdata;
+	enum port port;
 
 	if (!has_ddi_port_info(i915))
 		return;
 
 	list_for_each_entry(devdata, &i915->vbt.display_devices, node)
 		parse_ddi_port(devdata);
+
+	for_each_port(port) {
+		if (i915->vbt.ports[port])
+			print_ddi_port(i915->vbt.ports[port], port);
+	}
 }
 
 static void