All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
@ 2023-06-23  9:46 Jocelyn Falempe
  2023-07-04 14:19 ` Thomas Zimmermann
  2023-07-04 14:54 ` Jani Nikula
  0 siblings, 2 replies; 24+ messages in thread
From: Jocelyn Falempe @ 2023-06-23  9:46 UTC (permalink / raw)
  To: tzimmermann, airlied, kuohsiang_chou, jammy_huang
  Cc: Jocelyn Falempe, dri-devel

Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
 EDID on DP")
The default resolution is now 640x480 when no monitor is connected.
But Aspeed graphics is mostly used in servers, where no monitor
is attached. This also affects the "remote" resolution to 640x480, which is
inconvenient, and breaks the anaconda installer.
So when no EDID is present, set 1024x768 as preferred resolution.

Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")
Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
---
 drivers/gpu/drm/ast/ast_mode.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
index 36374828f6c8..8f7b7cc021c7 100644
--- a/drivers/gpu/drm/ast/ast_mode.c
+++ b/drivers/gpu/drm/ast/ast_mode.c
@@ -1589,9 +1589,31 @@ static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs
 	.get_modes = ast_dp501_connector_helper_get_modes,
 };
 
+static int ast_dp_probe_single_connector_modes(struct drm_connector *connector,
+					       uint32_t maxX, uint32_t maxY)
+{
+	int ret;
+	struct drm_display_mode *mode;
+
+	ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
+	/*
+	 * When no monitor are detected, DP now default to 640x480
+	 * As aspeed is mostly used in remote server, and DP monitors are
+	 * rarely attached, it's better to default to 1024x768
+	 */
+	if (!connector->edid_blob_ptr) {
+		list_for_each_entry(mode, &connector->modes, head) {
+			if (mode->hdisplay == 1024 && mode->vdisplay == 768)
+				mode->type |= DRM_MODE_TYPE_PREFERRED;
+			drm_mode_sort(&connector->modes);
+		}
+	}
+	return ret;
+}
+
 static const struct drm_connector_funcs ast_dp501_connector_funcs = {
 	.reset = drm_atomic_helper_connector_reset,
-	.fill_modes = drm_helper_probe_single_connector_modes,
+	.fill_modes = ast_dp_probe_single_connector_modes,
 	.destroy = drm_connector_cleanup,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
@@ -1678,7 +1700,7 @@ static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs
 
 static const struct drm_connector_funcs ast_astdp_connector_funcs = {
 	.reset = drm_atomic_helper_connector_reset,
-	.fill_modes = drm_helper_probe_single_connector_modes,
+	.fill_modes = ast_dp_probe_single_connector_modes,
 	.destroy = drm_connector_cleanup,
 	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,

base-commit: 0adec22702d497385dbdc52abb165f379a00efba
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-06-23  9:46 [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP Jocelyn Falempe
@ 2023-07-04 14:19 ` Thomas Zimmermann
  2023-07-04 14:21   ` Thomas Zimmermann
  2023-07-04 14:54 ` Jani Nikula
  1 sibling, 1 reply; 24+ messages in thread
From: Thomas Zimmermann @ 2023-07-04 14:19 UTC (permalink / raw)
  To: Jocelyn Falempe, airlied, kuohsiang_chou, jammy_huang; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 3773 bytes --]

Hi Jocelyn

Am 23.06.23 um 11:46 schrieb Jocelyn Falempe:
> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>   EDID on DP")
> The default resolution is now 640x480 when no monitor is connected.
> But Aspeed graphics is mostly used in servers, where no monitor
> is attached. This also affects the "remote" resolution to 640x480, which is
> inconvenient, and breaks the anaconda installer.

By "remote resolution", you mean the display mode that the BMC uses?

> So when no EDID is present, set 1024x768 as preferred resolution.
> 
> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")

This commit says that 640x480 is the designated failsafe mode if no EDID 
is available. Therefore, I think we should not override it 
unconditionally. The ast driver is no special in that case.

But I see why you're doing this change. I think any solution should be 
implemented in drm_helper_probe_single_connector_modes().

But before we solve this in the kernel, is it possible to delegate this 
to userspace? If no EDID has been given, userspace could try a 
non-failsafe display mode.

Best regards
Thomas

> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
>   drivers/gpu/drm/ast/ast_mode.c | 26 ++++++++++++++++++++++++--
>   1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 36374828f6c8..8f7b7cc021c7 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1589,9 +1589,31 @@ static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs
>   	.get_modes = ast_dp501_connector_helper_get_modes,
>   };
>   
> +static int ast_dp_probe_single_connector_modes(struct drm_connector *connector,
> +					       uint32_t maxX, uint32_t maxY)
> +{
> +	int ret;
> +	struct drm_display_mode *mode;
> +
> +	ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
> +	/*
> +	 * When no monitor are detected, DP now default to 640x480
> +	 * As aspeed is mostly used in remote server, and DP monitors are
> +	 * rarely attached, it's better to default to 1024x768
> +	 */
> +	if (!connector->edid_blob_ptr) {
> +		list_for_each_entry(mode, &connector->modes, head) {
> +			if (mode->hdisplay == 1024 && mode->vdisplay == 768)
> +				mode->type |= DRM_MODE_TYPE_PREFERRED;
> +			drm_mode_sort(&connector->modes);
> +		}
> +	}
> +	return ret;
> +}
> +
>   static const struct drm_connector_funcs ast_dp501_connector_funcs = {
>   	.reset = drm_atomic_helper_connector_reset,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.fill_modes = ast_dp_probe_single_connector_modes,
>   	.destroy = drm_connector_cleanup,
>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> @@ -1678,7 +1700,7 @@ static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs
>   
>   static const struct drm_connector_funcs ast_astdp_connector_funcs = {
>   	.reset = drm_atomic_helper_connector_reset,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.fill_modes = ast_dp_probe_single_connector_modes,
>   	.destroy = drm_connector_cleanup,
>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> 
> base-commit: 0adec22702d497385dbdc52abb165f379a00efba

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-04 14:19 ` Thomas Zimmermann
@ 2023-07-04 14:21   ` Thomas Zimmermann
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Zimmermann @ 2023-07-04 14:21 UTC (permalink / raw)
  To: Jocelyn Falempe, airlied, kuohsiang_chou, jammy_huang,
	Douglas Anderson, Abhinav Kumar, Dmitry Baryshkov, Jani Nikula,
	Sean Paul
  Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 4310 bytes --]

(cc: some people from commit fae7d186403e)

Am 04.07.23 um 16:19 schrieb Thomas Zimmermann:
> Hi Jocelyn
> 
> Am 23.06.23 um 11:46 schrieb Jocelyn Falempe:
>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>>   EDID on DP")
>> The default resolution is now 640x480 when no monitor is connected.
>> But Aspeed graphics is mostly used in servers, where no monitor
>> is attached. This also affects the "remote" resolution to 640x480, 
>> which is
>> inconvenient, and breaks the anaconda installer.
> 
> By "remote resolution", you mean the display mode that the BMC uses?
> 
>> So when no EDID is present, set 1024x768 as preferred resolution.
>>
>> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID 
>> on DP")
> 
> This commit says that 640x480 is the designated failsafe mode if no EDID 
> is available. Therefore, I think we should not override it 
> unconditionally. The ast driver is no special in that case.
> 
> But I see why you're doing this change. I think any solution should be 
> implemented in drm_helper_probe_single_connector_modes().
> 
> But before we solve this in the kernel, is it possible to delegate this 
> to userspace? If no EDID has been given, userspace could try a 
> non-failsafe display mode.
> 
> Best regards
> Thomas
> 
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> ---
>>   drivers/gpu/drm/ast/ast_mode.c | 26 ++++++++++++++++++++++++--
>>   1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c 
>> b/drivers/gpu/drm/ast/ast_mode.c
>> index 36374828f6c8..8f7b7cc021c7 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -1589,9 +1589,31 @@ static const struct drm_connector_helper_funcs 
>> ast_dp501_connector_helper_funcs
>>       .get_modes = ast_dp501_connector_helper_get_modes,
>>   };
>> +static int ast_dp_probe_single_connector_modes(struct drm_connector 
>> *connector,
>> +                           uint32_t maxX, uint32_t maxY)
>> +{
>> +    int ret;
>> +    struct drm_display_mode *mode;
>> +
>> +    ret = drm_helper_probe_single_connector_modes(connector, maxX, 
>> maxY);
>> +    /*
>> +     * When no monitor are detected, DP now default to 640x480
>> +     * As aspeed is mostly used in remote server, and DP monitors are
>> +     * rarely attached, it's better to default to 1024x768
>> +     */
>> +    if (!connector->edid_blob_ptr) {
>> +        list_for_each_entry(mode, &connector->modes, head) {
>> +            if (mode->hdisplay == 1024 && mode->vdisplay == 768)
>> +                mode->type |= DRM_MODE_TYPE_PREFERRED;
>> +            drm_mode_sort(&connector->modes);
>> +        }
>> +    }
>> +    return ret;
>> +}
>> +
>>   static const struct drm_connector_funcs ast_dp501_connector_funcs = {
>>       .reset = drm_atomic_helper_connector_reset,
>> -    .fill_modes = drm_helper_probe_single_connector_modes,
>> +    .fill_modes = ast_dp_probe_single_connector_modes,
>>       .destroy = drm_connector_cleanup,
>>       .atomic_duplicate_state = 
>> drm_atomic_helper_connector_duplicate_state,
>>       .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> @@ -1678,7 +1700,7 @@ static const struct drm_connector_helper_funcs 
>> ast_astdp_connector_helper_funcs
>>   static const struct drm_connector_funcs ast_astdp_connector_funcs = {
>>       .reset = drm_atomic_helper_connector_reset,
>> -    .fill_modes = drm_helper_probe_single_connector_modes,
>> +    .fill_modes = ast_dp_probe_single_connector_modes,
>>       .destroy = drm_connector_cleanup,
>>       .atomic_duplicate_state = 
>> drm_atomic_helper_connector_duplicate_state,
>>       .atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>
>> base-commit: 0adec22702d497385dbdc52abb165f379a00efba
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-06-23  9:46 [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP Jocelyn Falempe
  2023-07-04 14:19 ` Thomas Zimmermann
@ 2023-07-04 14:54 ` Jani Nikula
  2023-07-04 16:45   ` Jocelyn Falempe
  1 sibling, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2023-07-04 14:54 UTC (permalink / raw)
  To: Jocelyn Falempe, tzimmermann, airlied, kuohsiang_chou, jammy_huang
  Cc: Jocelyn Falempe, dri-devel

On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>  EDID on DP")
> The default resolution is now 640x480 when no monitor is connected.
> But Aspeed graphics is mostly used in servers, where no monitor
> is attached. This also affects the "remote" resolution to 640x480, which is
> inconvenient, and breaks the anaconda installer.
> So when no EDID is present, set 1024x768 as preferred resolution.

This conflates "monitor connected" and "EDID present", which are not
necessarily the same thing.

The fallback in drm_helper_probe_single_connector_modes() is for no
modes, but connector status is connected or unknown.

You could add a connector ->detect callback that returns disconnected
when there's no display, and the problem should go away. If there's no
->detect callback, it'll default to connected.

> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")
> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
> ---
>  drivers/gpu/drm/ast/ast_mode.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
> index 36374828f6c8..8f7b7cc021c7 100644
> --- a/drivers/gpu/drm/ast/ast_mode.c
> +++ b/drivers/gpu/drm/ast/ast_mode.c
> @@ -1589,9 +1589,31 @@ static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs
>  	.get_modes = ast_dp501_connector_helper_get_modes,
>  };
>  
> +static int ast_dp_probe_single_connector_modes(struct drm_connector *connector,
> +					       uint32_t maxX, uint32_t maxY)
> +{
> +	int ret;
> +	struct drm_display_mode *mode;
> +
> +	ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
> +	/*
> +	 * When no monitor are detected, DP now default to 640x480
> +	 * As aspeed is mostly used in remote server, and DP monitors are
> +	 * rarely attached, it's better to default to 1024x768
> +	 */
> +	if (!connector->edid_blob_ptr) {

Please try not to use connector->edid_blob_ptr for anything in
drivers. The logic is complicated enough as it is, with the firmware and
override EDIDs and everything, and makes future refactoring of EDID
handling harder.


BR,
Jani.

> +		list_for_each_entry(mode, &connector->modes, head) {
> +			if (mode->hdisplay == 1024 && mode->vdisplay == 768)
> +				mode->type |= DRM_MODE_TYPE_PREFERRED;
> +			drm_mode_sort(&connector->modes);
> +		}
> +	}
> +	return ret;
> +}
> +
>  static const struct drm_connector_funcs ast_dp501_connector_funcs = {
>  	.reset = drm_atomic_helper_connector_reset,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.fill_modes = ast_dp_probe_single_connector_modes,
>  	.destroy = drm_connector_cleanup,
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
> @@ -1678,7 +1700,7 @@ static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs
>  
>  static const struct drm_connector_funcs ast_astdp_connector_funcs = {
>  	.reset = drm_atomic_helper_connector_reset,
> -	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.fill_modes = ast_dp_probe_single_connector_modes,
>  	.destroy = drm_connector_cleanup,
>  	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>  	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>
> base-commit: 0adec22702d497385dbdc52abb165f379a00efba

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-04 14:54 ` Jani Nikula
@ 2023-07-04 16:45   ` Jocelyn Falempe
  2023-07-06  9:16     ` Jocelyn Falempe
  0 siblings, 1 reply; 24+ messages in thread
From: Jocelyn Falempe @ 2023-07-04 16:45 UTC (permalink / raw)
  To: Jani Nikula, tzimmermann, airlied, kuohsiang_chou, jammy_huang; +Cc: dri-devel

On 04/07/2023 16:54, Jani Nikula wrote:
> On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>>   EDID on DP")
>> The default resolution is now 640x480 when no monitor is connected.
>> But Aspeed graphics is mostly used in servers, where no monitor
>> is attached. This also affects the "remote" resolution to 640x480, which is
>> inconvenient, and breaks the anaconda installer.
>> So when no EDID is present, set 1024x768 as preferred resolution.
> 
> This conflates "monitor connected" and "EDID present", which are not
> necessarily the same thing.
> 
> The fallback in drm_helper_probe_single_connector_modes() is for no
> modes, but connector status is connected or unknown.

When debugging the issue, I found it surprising that the status is 
"connected" when nothing is plugged in the DP port.
> 
> You could add a connector ->detect callback that returns disconnected
> when there's no display, and the problem should go away. If there's no
> ->detect callback, it'll default to connected.

ok, I'll try that. I don't know how the hardware detect something is 
connected, but looking at the dp code, maybe checking 
"AST_IO_CRTC_PORT,0xDC, ASTDP_LINK_SUCCESS" would be good enough.

> 
>> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID on DP")
>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>> ---
>>   drivers/gpu/drm/ast/ast_mode.c | 26 ++++++++++++++++++++++++--
>>   1 file changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_mode.c b/drivers/gpu/drm/ast/ast_mode.c
>> index 36374828f6c8..8f7b7cc021c7 100644
>> --- a/drivers/gpu/drm/ast/ast_mode.c
>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>> @@ -1589,9 +1589,31 @@ static const struct drm_connector_helper_funcs ast_dp501_connector_helper_funcs
>>   	.get_modes = ast_dp501_connector_helper_get_modes,
>>   };
>>   
>> +static int ast_dp_probe_single_connector_modes(struct drm_connector *connector,
>> +					       uint32_t maxX, uint32_t maxY)
>> +{
>> +	int ret;
>> +	struct drm_display_mode *mode;
>> +
>> +	ret = drm_helper_probe_single_connector_modes(connector, maxX, maxY);
>> +	/*
>> +	 * When no monitor are detected, DP now default to 640x480
>> +	 * As aspeed is mostly used in remote server, and DP monitors are
>> +	 * rarely attached, it's better to default to 1024x768
>> +	 */
>> +	if (!connector->edid_blob_ptr) {
> 
> Please try not to use connector->edid_blob_ptr for anything in
> drivers. The logic is complicated enough as it is, with the firmware and
> override EDIDs and everything, and makes future refactoring of EDID
> handling harder.

Ok, I will try your other suggestion, and remove this.

Thanks a lot for your comments.

-- 

Jocelyn
> 
> 
> BR,
> Jani.
> 
>> +		list_for_each_entry(mode, &connector->modes, head) {
>> +			if (mode->hdisplay == 1024 && mode->vdisplay == 768)
>> +				mode->type |= DRM_MODE_TYPE_PREFERRED;
>> +			drm_mode_sort(&connector->modes);
>> +		}
>> +	}
>> +	return ret;
>> +}
>> +
>>   static const struct drm_connector_funcs ast_dp501_connector_funcs = {
>>   	.reset = drm_atomic_helper_connector_reset,
>> -	.fill_modes = drm_helper_probe_single_connector_modes,
>> +	.fill_modes = ast_dp_probe_single_connector_modes,
>>   	.destroy = drm_connector_cleanup,
>>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>> @@ -1678,7 +1700,7 @@ static const struct drm_connector_helper_funcs ast_astdp_connector_helper_funcs
>>   
>>   static const struct drm_connector_funcs ast_astdp_connector_funcs = {
>>   	.reset = drm_atomic_helper_connector_reset,
>> -	.fill_modes = drm_helper_probe_single_connector_modes,
>> +	.fill_modes = ast_dp_probe_single_connector_modes,
>>   	.destroy = drm_connector_cleanup,
>>   	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
>>   	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
>>
>> base-commit: 0adec22702d497385dbdc52abb165f379a00efba
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-04 16:45   ` Jocelyn Falempe
@ 2023-07-06  9:16     ` Jocelyn Falempe
  2023-07-06  9:32       ` Jani Nikula
  2023-07-06 10:26       ` Thomas Zimmermann
  0 siblings, 2 replies; 24+ messages in thread
From: Jocelyn Falempe @ 2023-07-06  9:16 UTC (permalink / raw)
  To: Jani Nikula, tzimmermann, airlied, kuohsiang_chou, jammy_huang; +Cc: dri-devel

On 04/07/2023 18:45, Jocelyn Falempe wrote:
> On 04/07/2023 16:54, Jani Nikula wrote:
>> On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>>>   EDID on DP")
>>> The default resolution is now 640x480 when no monitor is connected.
>>> But Aspeed graphics is mostly used in servers, where no monitor
>>> is attached. This also affects the "remote" resolution to 640x480, 
>>> which is
>>> inconvenient, and breaks the anaconda installer.
>>> So when no EDID is present, set 1024x768 as preferred resolution.
>>
>> This conflates "monitor connected" and "EDID present", which are not
>> necessarily the same thing.
>>
>> The fallback in drm_helper_probe_single_connector_modes() is for no
>> modes, but connector status is connected or unknown.
> 
> When debugging the issue, I found it surprising that the status is 
> "connected" when nothing is plugged in the DP port.
>>
>> You could add a connector ->detect callback that returns disconnected
>> when there's no display, and the problem should go away. If there's no
>> ->detect callback, it'll default to connected.
> 
> ok, I'll try that. I don't know how the hardware detect something is 
> connected, but looking at the dp code, maybe checking 
> "AST_IO_CRTC_PORT,0xDC, ASTDP_LINK_SUCCESS" would be good enough.

I've tested this approach, and it works. But on the server I'm testing, 
there are VGA and DP output. I think on a server that has only one DP 
output, if no monitor is connected, then no modes will be reported to 
userspace, and the remote BMC may not work ?

Also I don't have physical access to the server, so I only tested when 
no monitor is plugged.

I will send shortly a v2 with this change, so others can help me test 
this case.

Best regards,

-- 

Jocelyn


> 
>>
>>> Fixes: fae7d186403e ("drm/probe-helper: Default to 640x480 if no EDID 
>>> on DP")
>>> Signed-off-by: Jocelyn Falempe <jfalempe@redhat.com>
>>> ---
>>>   drivers/gpu/drm/ast/ast_mode.c | 26 ++++++++++++++++++++++++--
>>>   1 file changed, 24 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/ast/ast_mode.c 
>>> b/drivers/gpu/drm/ast/ast_mode.c
>>> index 36374828f6c8..8f7b7cc021c7 100644
>>> --- a/drivers/gpu/drm/ast/ast_mode.c
>>> +++ b/drivers/gpu/drm/ast/ast_mode.c
>>> @@ -1589,9 +1589,31 @@ static const struct drm_connector_helper_funcs 
>>> ast_dp501_connector_helper_funcs
>>>       .get_modes = ast_dp501_connector_helper_get_modes,
>>>   };
>>> +static int ast_dp_probe_single_connector_modes(struct drm_connector 
>>> *connector,
>>> +                           uint32_t maxX, uint32_t maxY)
>>> +{
>>> +    int ret;
>>> +    struct drm_display_mode *mode;
>>> +
>>> +    ret = drm_helper_probe_single_connector_modes(connector, maxX, 
>>> maxY);
>>> +    /*
>>> +     * When no monitor are detected, DP now default to 640x480
>>> +     * As aspeed is mostly used in remote server, and DP monitors are
>>> +     * rarely attached, it's better to default to 1024x768
>>> +     */
>>> +    if (!connector->edid_blob_ptr) {
>>
>> Please try not to use connector->edid_blob_ptr for anything in
>> drivers. The logic is complicated enough as it is, with the firmware and
>> override EDIDs and everything, and makes future refactoring of EDID
>> handling harder.
> 
> Ok, I will try your other suggestion, and remove this.
> 
> Thanks a lot for your comments.
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-06  9:16     ` Jocelyn Falempe
@ 2023-07-06  9:32       ` Jani Nikula
  2023-07-06 10:07         ` Jocelyn Falempe
  2023-07-06 10:26       ` Thomas Zimmermann
  1 sibling, 1 reply; 24+ messages in thread
From: Jani Nikula @ 2023-07-06  9:32 UTC (permalink / raw)
  To: Jocelyn Falempe, tzimmermann, airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel

On Thu, 06 Jul 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
> On 04/07/2023 18:45, Jocelyn Falempe wrote:
>> On 04/07/2023 16:54, Jani Nikula wrote:
>>> On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>>>>   EDID on DP")
>>>> The default resolution is now 640x480 when no monitor is connected.
>>>> But Aspeed graphics is mostly used in servers, where no monitor
>>>> is attached. This also affects the "remote" resolution to 640x480, 
>>>> which is
>>>> inconvenient, and breaks the anaconda installer.
>>>> So when no EDID is present, set 1024x768 as preferred resolution.
>>>
>>> This conflates "monitor connected" and "EDID present", which are not
>>> necessarily the same thing.
>>>
>>> The fallback in drm_helper_probe_single_connector_modes() is for no
>>> modes, but connector status is connected or unknown.
>> 
>> When debugging the issue, I found it surprising that the status is 
>> "connected" when nothing is plugged in the DP port.
>>>
>>> You could add a connector ->detect callback that returns disconnected
>>> when there's no display, and the problem should go away. If there's no
>>> ->detect callback, it'll default to connected.
>> 
>> ok, I'll try that. I don't know how the hardware detect something is 
>> connected, but looking at the dp code, maybe checking 
>> "AST_IO_CRTC_PORT,0xDC, ASTDP_LINK_SUCCESS" would be good enough.
>
> I've tested this approach, and it works.

\o/

> But on the server I'm testing, 
> there are VGA and DP output. I think on a server that has only one DP 
> output, if no monitor is connected, then no modes will be reported to 
> userspace, and the remote BMC may not work ?

I couldn't say, but having the driver lie about the connected status to
make it work feels wrong.

> Also I don't have physical access to the server, so I only tested when 
> no monitor is plugged.
>
> I will send shortly a v2 with this change, so others can help me test 
> this case.

Thanks,
Jani.


>
> Best regards,

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-06  9:32       ` Jani Nikula
@ 2023-07-06 10:07         ` Jocelyn Falempe
  2023-07-06 10:14           ` Jani Nikula
  0 siblings, 1 reply; 24+ messages in thread
From: Jocelyn Falempe @ 2023-07-06 10:07 UTC (permalink / raw)
  To: Jani Nikula, tzimmermann, airlied, kuohsiang_chou, jammy_huang; +Cc: dri-devel

On 06/07/2023 11:32, Jani Nikula wrote:
> On Thu, 06 Jul 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
>> On 04/07/2023 18:45, Jocelyn Falempe wrote:
>>> On 04/07/2023 16:54, Jani Nikula wrote:
>>>> On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>>>>>    EDID on DP")
>>>>> The default resolution is now 640x480 when no monitor is connected.
>>>>> But Aspeed graphics is mostly used in servers, where no monitor
>>>>> is attached. This also affects the "remote" resolution to 640x480,
>>>>> which is
>>>>> inconvenient, and breaks the anaconda installer.
>>>>> So when no EDID is present, set 1024x768 as preferred resolution.
>>>>
>>>> This conflates "monitor connected" and "EDID present", which are not
>>>> necessarily the same thing.
>>>>
>>>> The fallback in drm_helper_probe_single_connector_modes() is for no
>>>> modes, but connector status is connected or unknown.
>>>
>>> When debugging the issue, I found it surprising that the status is
>>> "connected" when nothing is plugged in the DP port.
>>>>
>>>> You could add a connector ->detect callback that returns disconnected
>>>> when there's no display, and the problem should go away. If there's no
>>>> ->detect callback, it'll default to connected.
>>>
>>> ok, I'll try that. I don't know how the hardware detect something is
>>> connected, but looking at the dp code, maybe checking
>>> "AST_IO_CRTC_PORT,0xDC, ASTDP_LINK_SUCCESS" would be good enough.
>>
>> I've tested this approach, and it works.
> 
> \o/
> 
>> But on the server I'm testing,
>> there are VGA and DP output. I think on a server that has only one DP
>> output, if no monitor is connected, then no modes will be reported to
>> userspace, and the remote BMC may not work ?
> 
> I couldn't say, but having the driver lie about the connected status to
> make it work feels wrong.

Yes, so maybe a better way would be to add a remote/bmc connector, with 
proper default resolution ?
That will better reflect what the hardware does.

-- 

Jocelyn

> 
>> Also I don't have physical access to the server, so I only tested when
>> no monitor is plugged.
>>
>> I will send shortly a v2 with this change, so others can help me test
>> this case.
> 
> Thanks,
> Jani.
> 
> 
>>
>> Best regards,
> 



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-06 10:07         ` Jocelyn Falempe
@ 2023-07-06 10:14           ` Jani Nikula
  0 siblings, 0 replies; 24+ messages in thread
From: Jani Nikula @ 2023-07-06 10:14 UTC (permalink / raw)
  To: Jocelyn Falempe, tzimmermann, airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel

On Thu, 06 Jul 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
> Yes, so maybe a better way would be to add a remote/bmc connector, with 
> proper default resolution ?
> That will better reflect what the hardware does.

I'm afraid I don't know enough about the hardware or use case to say.

BR,
Jani.


-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-06  9:16     ` Jocelyn Falempe
  2023-07-06  9:32       ` Jani Nikula
@ 2023-07-06 10:26       ` Thomas Zimmermann
  2023-07-06 11:31         ` Jocelyn Falempe
  1 sibling, 1 reply; 24+ messages in thread
From: Thomas Zimmermann @ 2023-07-06 10:26 UTC (permalink / raw)
  To: Jocelyn Falempe, Jani Nikula, airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2288 bytes --]

Hi

Am 06.07.23 um 11:16 schrieb Jocelyn Falempe:
> On 04/07/2023 18:45, Jocelyn Falempe wrote:
>> On 04/07/2023 16:54, Jani Nikula wrote:
>>> On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>>>>   EDID on DP")
>>>> The default resolution is now 640x480 when no monitor is connected.
>>>> But Aspeed graphics is mostly used in servers, where no monitor
>>>> is attached. This also affects the "remote" resolution to 640x480, 
>>>> which is
>>>> inconvenient, and breaks the anaconda installer.
>>>> So when no EDID is present, set 1024x768 as preferred resolution.
>>>
>>> This conflates "monitor connected" and "EDID present", which are not
>>> necessarily the same thing.
>>>
>>> The fallback in drm_helper_probe_single_connector_modes() is for no
>>> modes, but connector status is connected or unknown.
>>
>> When debugging the issue, I found it surprising that the status is 
>> "connected" when nothing is plugged in the DP port.
>>>
>>> You could add a connector ->detect callback that returns disconnected
>>> when there's no display, and the problem should go away. If there's no
>>> ->detect callback, it'll default to connected.
>>
>> ok, I'll try that. I don't know how the hardware detect something is 
>> connected, but looking at the dp code, maybe checking 
>> "AST_IO_CRTC_PORT,0xDC, ASTDP_LINK_SUCCESS" would be good enough.
> 
> I've tested this approach, and it works. But on the server I'm testing, 
> there are VGA and DP output. I think on a server that has only one DP 
> output, if no monitor is connected, then no modes will be reported to 
> userspace, and the remote BMC may not work ?

You could out-comment the VGA code in the ast driver for testing.

Best regards
Thomas

> 
> Also I don't have physical access to the server, so I only tested when 
> no monitor is plugged.
> 
> I will send shortly a v2 with this change, so others can help me test 
> this case.
> 
> Best regards,
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-06 10:26       ` Thomas Zimmermann
@ 2023-07-06 11:31         ` Jocelyn Falempe
  2023-07-06 16:37           ` Jocelyn Falempe
  0 siblings, 1 reply; 24+ messages in thread
From: Jocelyn Falempe @ 2023-07-06 11:31 UTC (permalink / raw)
  To: Thomas Zimmermann, Jani Nikula, airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel

On 06/07/2023 12:26, Thomas Zimmermann wrote:
> Hi
> 
> Am 06.07.23 um 11:16 schrieb Jocelyn Falempe:
>> On 04/07/2023 18:45, Jocelyn Falempe wrote:
>>> On 04/07/2023 16:54, Jani Nikula wrote:
>>>> On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 if no
>>>>>   EDID on DP")
>>>>> The default resolution is now 640x480 when no monitor is connected.
>>>>> But Aspeed graphics is mostly used in servers, where no monitor
>>>>> is attached. This also affects the "remote" resolution to 640x480, 
>>>>> which is
>>>>> inconvenient, and breaks the anaconda installer.
>>>>> So when no EDID is present, set 1024x768 as preferred resolution.
>>>>
>>>> This conflates "monitor connected" and "EDID present", which are not
>>>> necessarily the same thing.
>>>>
>>>> The fallback in drm_helper_probe_single_connector_modes() is for no
>>>> modes, but connector status is connected or unknown.
>>>
>>> When debugging the issue, I found it surprising that the status is 
>>> "connected" when nothing is plugged in the DP port.
>>>>
>>>> You could add a connector ->detect callback that returns disconnected
>>>> when there's no display, and the problem should go away. If there's no
>>>> ->detect callback, it'll default to connected.
>>>
>>> ok, I'll try that. I don't know how the hardware detect something is 
>>> connected, but looking at the dp code, maybe checking 
>>> "AST_IO_CRTC_PORT,0xDC, ASTDP_LINK_SUCCESS" would be good enough.
>>
>> I've tested this approach, and it works. But on the server I'm 
>> testing, there are VGA and DP output. I think on a server that has 
>> only one DP output, if no monitor is connected, then no modes will be 
>> reported to userspace, and the remote BMC may not work ?
> 
> You could out-comment the VGA code in the ast driver for testing.

Oh, Thanks for the idea, I will try that.

-- 

Jocelyn
> 
> Best regards
> Thomas
> 
>>
>> Also I don't have physical access to the server, so I only tested when 
>> no monitor is plugged.
>>
>> I will send shortly a v2 with this change, so others can help me test 
>> this case.
>>
>> Best regards,
>>
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-06 11:31         ` Jocelyn Falempe
@ 2023-07-06 16:37           ` Jocelyn Falempe
  2023-07-07  7:30             ` Thomas Zimmermann
  0 siblings, 1 reply; 24+ messages in thread
From: Jocelyn Falempe @ 2023-07-06 16:37 UTC (permalink / raw)
  To: Thomas Zimmermann, Jani Nikula, airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel

On 06/07/2023 13:31, Jocelyn Falempe wrote:
> On 06/07/2023 12:26, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 06.07.23 um 11:16 schrieb Jocelyn Falempe:
>>> On 04/07/2023 18:45, Jocelyn Falempe wrote:
>>>> On 04/07/2023 16:54, Jani Nikula wrote:
>>>>> On Fri, 23 Jun 2023, Jocelyn Falempe <jfalempe@redhat.com> wrote:
>>>>>> Since commit fae7d186403e ("drm/probe-helper: Default to 640x480 
>>>>>> if no
>>>>>>   EDID on DP")
>>>>>> The default resolution is now 640x480 when no monitor is connected.
>>>>>> But Aspeed graphics is mostly used in servers, where no monitor
>>>>>> is attached. This also affects the "remote" resolution to 640x480, 
>>>>>> which is
>>>>>> inconvenient, and breaks the anaconda installer.
>>>>>> So when no EDID is present, set 1024x768 as preferred resolution.
>>>>>
>>>>> This conflates "monitor connected" and "EDID present", which are not
>>>>> necessarily the same thing.
>>>>>
>>>>> The fallback in drm_helper_probe_single_connector_modes() is for no
>>>>> modes, but connector status is connected or unknown.
>>>>
>>>> When debugging the issue, I found it surprising that the status is 
>>>> "connected" when nothing is plugged in the DP port.
>>>>>
>>>>> You could add a connector ->detect callback that returns disconnected
>>>>> when there's no display, and the problem should go away. If there's no
>>>>> ->detect callback, it'll default to connected.
>>>>
>>>> ok, I'll try that. I don't know how the hardware detect something is 
>>>> connected, but looking at the dp code, maybe checking 
>>>> "AST_IO_CRTC_PORT,0xDC, ASTDP_LINK_SUCCESS" would be good enough.
>>>
>>> I've tested this approach, and it works. But on the server I'm 
>>> testing, there are VGA and DP output. I think on a server that has 
>>> only one DP output, if no monitor is connected, then no modes will be 
>>> reported to userspace, and the remote BMC may not work ?
>>
>> You could out-comment the VGA code in the ast driver for testing.
> 
> Oh, Thanks for the idea, I will try that.
> 

The result is that I get a black screen on the remote BMC. So maybe 
adding a remote/bmc connector will solve that.


-- 

Jocelyn


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-06 16:37           ` Jocelyn Falempe
@ 2023-07-07  7:30             ` Thomas Zimmermann
  2023-07-10  8:07               ` Jocelyn Falempe
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Zimmermann @ 2023-07-07  7:30 UTC (permalink / raw)
  To: Jocelyn Falempe, Jani Nikula, airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 723 bytes --]

Hi

Am 06.07.23 um 18:37 schrieb Jocelyn Falempe:
[...]
>>>
>>> You could out-comment the VGA code in the ast driver for testing.
>>
>> Oh, Thanks for the idea, I will try that.
>>
> 
> The result is that I get a black screen on the remote BMC. So maybe 
> adding a remote/bmc connector will solve that.

Could work. That would be a connector and encoder of type 'virtual'?

Not all ast devices have a BMC. Do you know how to detect its presence?

Best regards
Thomas

> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-07  7:30             ` Thomas Zimmermann
@ 2023-07-10  8:07               ` Jocelyn Falempe
  2023-07-12 15:05                 ` Sui Jingfeng
  0 siblings, 1 reply; 24+ messages in thread
From: Jocelyn Falempe @ 2023-07-10  8:07 UTC (permalink / raw)
  To: Thomas Zimmermann, Jani Nikula, airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel

On 07/07/2023 09:30, Thomas Zimmermann wrote:
> Hi
> 
> Am 06.07.23 um 18:37 schrieb Jocelyn Falempe:
> [...]
>>>>
>>>> You could out-comment the VGA code in the ast driver for testing.
>>>
>>> Oh, Thanks for the idea, I will try that.
>>>
>>
>> The result is that I get a black screen on the remote BMC. So maybe 
>> adding a remote/bmc connector will solve that.
> 
> Could work. That would be a connector and encoder of type 'virtual'?
> 
> Not all ast devices have a BMC. Do you know how to detect its presence?

Hum, I though all ast devices have a BMC, and I don't see a way to 
detect that BMC is active or present. (It would be even better to know 
the browser's size, and automatically resize, like when using a VM. But 
I'm not sure the hardware/firmware is able to do this).

On the other hand, are there any drawback to present a BMC connector 
even when the hardware doesn't have it ?
> 
> Best regards
> Thomas
> 
>>
>>
> 

Best regards,

-- 

Jocelyn


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-10  8:07               ` Jocelyn Falempe
@ 2023-07-12 15:05                 ` Sui Jingfeng
  2023-07-12 15:25                   ` Jocelyn Falempe
  2023-07-13  8:50                   ` Thomas Zimmermann
  0 siblings, 2 replies; 24+ messages in thread
From: Sui Jingfeng @ 2023-07-12 15:05 UTC (permalink / raw)
  To: Jocelyn Falempe, Thomas Zimmermann, Jani Nikula, airlied,
	kuohsiang_chou, jammy_huang
  Cc: dri-devel

Hi,


I'm here join to the discussion. Because I know a little about aspeed BMC.


On 2023/7/10 16:07, Jocelyn Falempe wrote:
> On 07/07/2023 09:30, Thomas Zimmermann wrote:
>> Hi
>>
>> Am 06.07.23 um 18:37 schrieb Jocelyn Falempe:
>> [...]
>>>>>
>>>>> You could out-comment the VGA code in the ast driver for testing.
>>>>
>>>> Oh, Thanks for the idea, I will try that.
>>>>
>>>
>>> The result is that I get a black screen on the remote BMC. So maybe 
>>> adding a remote/bmc connector will solve that.
>>
>> Could work. That would be a connector and encoder of type 'virtual'?
>>
>> Not all ast devices have a BMC. Do you know how to detect its presence?
>
> Hum, I though all ast devices have a BMC, 

No, Thomas is right, not all ast devices have a BMC.

I have two discrete AST BMC cards, see [1] for reference.

I generally using the ast2400 BMC cards to testing patches and study 
drm/ast driver.

It seems that this two cards are pure 2D display card, because they 
don't have a net interface(can not plug-in the net cable).


[1] 
https://github.com/loongson-gfx/loongson_boards/blob/main/ast_bmc_cards/ast1400_and_ast2400.jpg


> and I don't see a way to detect that BMC is active or present.

I think we better find one, then if the BMC is active (present).

we could create a virtual encoder and connector safely.


> (It would be even better to know the browser's size, and automatically 
> resize, like when using a VM. But I'm not sure the hardware/firmware 
> is able to do this).
>

I think it is not difficult, it just that need the firmware of your 
board to set a value to a register,

(a scratch register) or something like that.

But this really need you have the firmware (source code) to support this.

Or you are luckily, if there somebody already done this for you.

> On the other hand, are there any drawback to present a BMC connector 
> even when the hardware doesn't have it ?

If not properly setting up, I think you will create two encoder and two 
connector in the system.

>>
>> Best regards
>> Thomas
>>
>>>
>>>
>>
>
> Best regards,
>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-12 15:05                 ` Sui Jingfeng
@ 2023-07-12 15:25                   ` Jocelyn Falempe
  2023-07-13  8:32                     ` Michel Dänzer
  2023-07-13  8:50                   ` Thomas Zimmermann
  1 sibling, 1 reply; 24+ messages in thread
From: Jocelyn Falempe @ 2023-07-12 15:25 UTC (permalink / raw)
  To: Sui Jingfeng, Thomas Zimmermann, Jani Nikula, airlied,
	kuohsiang_chou, jammy_huang
  Cc: dri-devel

On 12/07/2023 17:05, Sui Jingfeng wrote:
> Hi,
> 
> 
> I'm here join to the discussion. Because I know a little about aspeed BMC.
> 
> 
> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>> On 07/07/2023 09:30, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 06.07.23 um 18:37 schrieb Jocelyn Falempe:
>>> [...]
>>>>>>
>>>>>> You could out-comment the VGA code in the ast driver for testing.
>>>>>
>>>>> Oh, Thanks for the idea, I will try that.
>>>>>
>>>>
>>>> The result is that I get a black screen on the remote BMC. So maybe 
>>>> adding a remote/bmc connector will solve that.
>>>
>>> Could work. That would be a connector and encoder of type 'virtual'?
>>>
>>> Not all ast devices have a BMC. Do you know how to detect its presence?
>>
>> Hum, I though all ast devices have a BMC, 
> 
> No, Thomas is right, not all ast devices have a BMC.
> 
> I have two discrete AST BMC cards, see [1] for reference.
> 
> I generally using the ast2400 BMC cards to testing patches and study 
> drm/ast driver.
> 
> It seems that this two cards are pure 2D display card, because they 
> don't have a net interface(can not plug-in the net cable).
> 
> 
> [1] 
> https://github.com/loongson-gfx/loongson_boards/blob/main/ast_bmc_cards/ast1400_and_ast2400.jpg

Thanks for this picture, I didn't know about this discrete graphic 
cards, with PCIe connector.

> 
> 
>> and I don't see a way to detect that BMC is active or present.
> 
> I think we better find one, then if the BMC is active (present).
> 
> we could create a virtual encoder and connector safely.
> 
> 
>> (It would be even better to know the browser's size, and automatically 
>> resize, like when using a VM. But I'm not sure the hardware/firmware 
>> is able to do this).
>>
> 
> I think it is not difficult, it just that need the firmware of your 
> board to set a value to a register,
> 
> (a scratch register) or something like that.
> 
> But this really need you have the firmware (source code) to support this.

Yes, that's the difficult part.
> 
> Or you are luckily, if there somebody already done this for you.
> 
>> On the other hand, are there any drawback to present a BMC connector 
>> even when the hardware doesn't have it ?
> 
> If not properly setting up, I think you will create two encoder and two 
> connector in the system.

Yes, but I think it won't have any visible effect for the end-user.

-- 

Jocelyn

> 
>>>
>>> Best regards
>>> Thomas
>>>
>>>>
>>>>
>>>
>>
>> Best regards,
>>
> 


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-12 15:25                   ` Jocelyn Falempe
@ 2023-07-13  8:32                     ` Michel Dänzer
  2023-07-13  8:49                       ` Thomas Zimmermann
  2023-07-13  8:53                       ` Jocelyn Falempe
  0 siblings, 2 replies; 24+ messages in thread
From: Michel Dänzer @ 2023-07-13  8:32 UTC (permalink / raw)
  To: Jocelyn Falempe, Sui Jingfeng, Thomas Zimmermann, Jani Nikula,
	airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel

On 7/12/23 17:25, Jocelyn Falempe wrote:
> On 12/07/2023 17:05, Sui Jingfeng wrote:
>> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>>
>>> On the other hand, are there any drawback to present a BMC connector even when the hardware doesn't have it ?
>>
>> If not properly setting up, I think you will create two encoder and two connector in the system.
> 
> Yes, but I think it won't have any visible effect for the end-user.

I'm afraid user-space display servers would waste effort producing content for a non-existing BMC (assuming its connector status is connected or unknown).


-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-13  8:32                     ` Michel Dänzer
@ 2023-07-13  8:49                       ` Thomas Zimmermann
  2023-07-13  8:53                         ` Michel Dänzer
  2023-07-13  8:53                       ` Jocelyn Falempe
  1 sibling, 1 reply; 24+ messages in thread
From: Thomas Zimmermann @ 2023-07-13  8:49 UTC (permalink / raw)
  To: Michel Dänzer, Jocelyn Falempe, Sui Jingfeng, Jani Nikula,
	airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1023 bytes --]

Hi

Am 13.07.23 um 10:32 schrieb Michel Dänzer:
> On 7/12/23 17:25, Jocelyn Falempe wrote:
>> On 12/07/2023 17:05, Sui Jingfeng wrote:
>>> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>>>
>>>> On the other hand, are there any drawback to present a BMC connector even when the hardware doesn't have it ?
>>>
>>> If not properly setting up, I think you will create two encoder and two connector in the system.
>>
>> Yes, but I think it won't have any visible effect for the end-user.
> 
> I'm afraid user-space display servers would waste effort producing content for a non-existing BMC (assuming its connector status is connected or unknown).

Right now, the BMC output works because the VGA status is always 
connected. So nothing really changes.

Best regards
Thomas

> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-12 15:05                 ` Sui Jingfeng
  2023-07-12 15:25                   ` Jocelyn Falempe
@ 2023-07-13  8:50                   ` Thomas Zimmermann
  1 sibling, 0 replies; 24+ messages in thread
From: Thomas Zimmermann @ 2023-07-13  8:50 UTC (permalink / raw)
  To: Sui Jingfeng, Jocelyn Falempe, Jani Nikula, airlied,
	kuohsiang_chou, jammy_huang
  Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 2602 bytes --]

Hi

Am 12.07.23 um 17:05 schrieb Sui Jingfeng:
> Hi,
> 
> 
> I'm here join to the discussion. Because I know a little about aspeed BMC.
> 
> 
> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>> On 07/07/2023 09:30, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 06.07.23 um 18:37 schrieb Jocelyn Falempe:
>>> [...]
>>>>>>
>>>>>> You could out-comment the VGA code in the ast driver for testing.
>>>>>
>>>>> Oh, Thanks for the idea, I will try that.
>>>>>
>>>>
>>>> The result is that I get a black screen on the remote BMC. So maybe 
>>>> adding a remote/bmc connector will solve that.
>>>
>>> Could work. That would be a connector and encoder of type 'virtual'?
>>>
>>> Not all ast devices have a BMC. Do you know how to detect its presence?
>>
>> Hum, I though all ast devices have a BMC, 
> 
> No, Thomas is right, not all ast devices have a BMC.
> 
> I have two discrete AST BMC cards, see [1] for reference.
> 
> I generally using the ast2400 BMC cards to testing patches and study 
> drm/ast driver.
> 
> It seems that this two cards are pure 2D display card, because they 
> don't have a net interface(can not plug-in the net cable).
> 
> 
> [1] 
> https://github.com/loongson-gfx/loongson_boards/blob/main/ast_bmc_cards/ast1400_and_ast2400.jpg

Thanks for the info.

Best regards
Thomas

> 
> 
>> and I don't see a way to detect that BMC is active or present.
> 
> I think we better find one, then if the BMC is active (present).
> 
> we could create a virtual encoder and connector safely.
> 
> 
>> (It would be even better to know the browser's size, and automatically 
>> resize, like when using a VM. But I'm not sure the hardware/firmware 
>> is able to do this).
>>
> 
> I think it is not difficult, it just that need the firmware of your 
> board to set a value to a register,
> 
> (a scratch register) or something like that.
> 
> But this really need you have the firmware (source code) to support this.
> 
> Or you are luckily, if there somebody already done this for you.
> 
>> On the other hand, are there any drawback to present a BMC connector 
>> even when the hardware doesn't have it ?
> 
> If not properly setting up, I think you will create two encoder and two 
> connector in the system.
> 
>>>
>>> Best regards
>>> Thomas
>>>
>>>>
>>>>
>>>
>>
>> Best regards,
>>
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-13  8:32                     ` Michel Dänzer
  2023-07-13  8:49                       ` Thomas Zimmermann
@ 2023-07-13  8:53                       ` Jocelyn Falempe
  2023-07-13  8:55                         ` Michel Dänzer
  1 sibling, 1 reply; 24+ messages in thread
From: Jocelyn Falempe @ 2023-07-13  8:53 UTC (permalink / raw)
  To: dri-devel

On 13/07/2023 10:32, Michel Dänzer wrote:
> On 7/12/23 17:25, Jocelyn Falempe wrote:
>> On 12/07/2023 17:05, Sui Jingfeng wrote:
>>> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>>>
>>>> On the other hand, are there any drawback to present a BMC connector even when the hardware doesn't have it ?
>>>
>>> If not properly setting up, I think you will create two encoder and two connector in the system.
>>
>> Yes, but I think it won't have any visible effect for the end-user.
> 
> I'm afraid user-space display servers would waste effort producing content for a non-existing BMC (assuming its connector status is connected or unknown).
> 
> 
I think it's already the case, as AST's DP and VGA connectors currently 
always report "connected" status. And they all share the same crtc, so 
there is only one framebuffer, that is always active.

Best regards,

-- 

Jocelyn


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-13  8:49                       ` Thomas Zimmermann
@ 2023-07-13  8:53                         ` Michel Dänzer
  2023-07-13  9:09                           ` Thomas Zimmermann
  0 siblings, 1 reply; 24+ messages in thread
From: Michel Dänzer @ 2023-07-13  8:53 UTC (permalink / raw)
  To: Thomas Zimmermann, Jocelyn Falempe, Sui Jingfeng, Jani Nikula,
	airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel

On 7/13/23 10:49, Thomas Zimmermann wrote:
> Am 13.07.23 um 10:32 schrieb Michel Dänzer:
>> On 7/12/23 17:25, Jocelyn Falempe wrote:
>>> On 12/07/2023 17:05, Sui Jingfeng wrote:
>>>> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>>>>
>>>>> On the other hand, are there any drawback to present a BMC connector even when the hardware doesn't have it ?
>>>>
>>>> If not properly setting up, I think you will create two encoder and two connector in the system.
>>>
>>> Yes, but I think it won't have any visible effect for the end-user.
>>
>> I'm afraid user-space display servers would waste effort producing content for a non-existing BMC (assuming its connector status is connected or unknown).
> 
> Right now, the BMC output works because the VGA status is always connected. So nothing really changes.

User-space display servers would generally produce different contents by default for the VGA & BMC connectors.


-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-13  8:53                       ` Jocelyn Falempe
@ 2023-07-13  8:55                         ` Michel Dänzer
  0 siblings, 0 replies; 24+ messages in thread
From: Michel Dänzer @ 2023-07-13  8:55 UTC (permalink / raw)
  To: Jocelyn Falempe; +Cc: dri-devel

On 7/13/23 10:53, Jocelyn Falempe wrote:
> On 13/07/2023 10:32, Michel Dänzer wrote:
>> On 7/12/23 17:25, Jocelyn Falempe wrote:
>>> On 12/07/2023 17:05, Sui Jingfeng wrote:
>>>> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>>>>
>>>>> On the other hand, are there any drawback to present a BMC connector even when the hardware doesn't have it ?
>>>>
>>>> If not properly setting up, I think you will create two encoder and two connector in the system.
>>>
>>> Yes, but I think it won't have any visible effect for the end-user.
>>
>> I'm afraid user-space display servers would waste effort producing content for a non-existing BMC (assuming its connector status is connected or unknown).
>>
>>
> I think it's already the case, as AST's DP and VGA connectors currently always report "connected" status. And they all share the same crtc, so there is only one framebuffer, that is always active.

"Single CRTC" is the piece of information I was missing. Never mind then.


-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-13  8:53                         ` Michel Dänzer
@ 2023-07-13  9:09                           ` Thomas Zimmermann
  2023-07-13  9:41                             ` Michel Dänzer
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Zimmermann @ 2023-07-13  9:09 UTC (permalink / raw)
  To: Michel Dänzer, Jocelyn Falempe, Sui Jingfeng, Jani Nikula,
	airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1287 bytes --]

Hi

Am 13.07.23 um 10:53 schrieb Michel Dänzer:
> On 7/13/23 10:49, Thomas Zimmermann wrote:
>> Am 13.07.23 um 10:32 schrieb Michel Dänzer:
>>> On 7/12/23 17:25, Jocelyn Falempe wrote:
>>>> On 12/07/2023 17:05, Sui Jingfeng wrote:
>>>>> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>>>>>
>>>>>> On the other hand, are there any drawback to present a BMC connector even when the hardware doesn't have it ?
>>>>>
>>>>> If not properly setting up, I think you will create two encoder and two connector in the system.
>>>>
>>>> Yes, but I think it won't have any visible effect for the end-user.
>>>
>>> I'm afraid user-space display servers would waste effort producing content for a non-existing BMC (assuming its connector status is connected or unknown).
>>
>> Right now, the BMC output works because the VGA status is always connected. So nothing really changes.
> 
> User-space display servers would generally produce different contents by default for the VGA & BMC connectors.

Can you elaborate? How would the output differ?

> 
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP
  2023-07-13  9:09                           ` Thomas Zimmermann
@ 2023-07-13  9:41                             ` Michel Dänzer
  0 siblings, 0 replies; 24+ messages in thread
From: Michel Dänzer @ 2023-07-13  9:41 UTC (permalink / raw)
  To: Thomas Zimmermann, Jocelyn Falempe, Sui Jingfeng, Jani Nikula,
	airlied, kuohsiang_chou, jammy_huang
  Cc: dri-devel

On 7/13/23 11:09, Thomas Zimmermann wrote:
> Am 13.07.23 um 10:53 schrieb Michel Dänzer:
>> On 7/13/23 10:49, Thomas Zimmermann wrote:
>>> Am 13.07.23 um 10:32 schrieb Michel Dänzer:
>>>> On 7/12/23 17:25, Jocelyn Falempe wrote:
>>>>> On 12/07/2023 17:05, Sui Jingfeng wrote:
>>>>>> On 2023/7/10 16:07, Jocelyn Falempe wrote:
>>>>>>
>>>>>>> On the other hand, are there any drawback to present a BMC connector even when the hardware doesn't have it ?
>>>>>>
>>>>>> If not properly setting up, I think you will create two encoder and two connector in the system.
>>>>>
>>>>> Yes, but I think it won't have any visible effect for the end-user.
>>>>
>>>> I'm afraid user-space display servers would waste effort producing content for a non-existing BMC (assuming its connector status is connected or unknown).
>>>
>>> Right now, the BMC output works because the VGA status is always connected. So nothing really changes.
>>
>> User-space display servers would generally produce different contents by default for the VGA & BMC connectors.
> 
> Can you elaborate? How would the output differ?

Per the other sub-thread, I didn't realize there's only a single CRTC.


-- 
Earthling Michel Dänzer            |                  https://redhat.com
Libre software enthusiast          |         Mesa and Xwayland developer


^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2023-07-13  9:41 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-23  9:46 [PATCH] drm/ast: Fix default resolution when no monitor is connected on DP Jocelyn Falempe
2023-07-04 14:19 ` Thomas Zimmermann
2023-07-04 14:21   ` Thomas Zimmermann
2023-07-04 14:54 ` Jani Nikula
2023-07-04 16:45   ` Jocelyn Falempe
2023-07-06  9:16     ` Jocelyn Falempe
2023-07-06  9:32       ` Jani Nikula
2023-07-06 10:07         ` Jocelyn Falempe
2023-07-06 10:14           ` Jani Nikula
2023-07-06 10:26       ` Thomas Zimmermann
2023-07-06 11:31         ` Jocelyn Falempe
2023-07-06 16:37           ` Jocelyn Falempe
2023-07-07  7:30             ` Thomas Zimmermann
2023-07-10  8:07               ` Jocelyn Falempe
2023-07-12 15:05                 ` Sui Jingfeng
2023-07-12 15:25                   ` Jocelyn Falempe
2023-07-13  8:32                     ` Michel Dänzer
2023-07-13  8:49                       ` Thomas Zimmermann
2023-07-13  8:53                         ` Michel Dänzer
2023-07-13  9:09                           ` Thomas Zimmermann
2023-07-13  9:41                             ` Michel Dänzer
2023-07-13  8:53                       ` Jocelyn Falempe
2023-07-13  8:55                         ` Michel Dänzer
2023-07-13  8:50                   ` Thomas Zimmermann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.