[Why]
External displays take priority over internal display when there are fewer
display controllers than displays.
[How]
The root cause is because of that number of the crtc is not correct.
The number of the crtc on the 3250c is 3, but on the 3500c is 4.
On the source code, we can see that number of the crtc has been fixed at 4.
Needs to set the num_crtc to 3 for 3250c platform.
Signed-off-by: RyanLin <[email protected]>
Change-Id: I837df7101cc4849d2c3021fd529b4061edab4bb1
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 40c91b448f7d..dbeef7b57a9b 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
break;
#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
case CHIP_RAVEN:
- adev->mode_info.num_crtc = 4;
- adev->mode_info.num_hpd = 4;
- adev->mode_info.num_dig = 4;
+ if (adev->rev_id >= 8) { //chip_name = "raven2";
+ adev->mode_info.num_crtc = 3;
+ adev->mode_info.num_hpd = 3;
+ adev->mode_info.num_dig = 3;
+ } else {
+ adev->mode_info.num_crtc = 4;
+ adev->mode_info.num_hpd = 4;
+ adev->mode_info.num_dig = 4;
+ }
break;
#endif
#if defined(CONFIG_DRM_AMD_DC_DCN2_0)
--
2.25.1
Please use C style comments /* */. WIth that fixed:
Reviewed-by: Alex Deucher <[email protected]>
On Thu, Jan 27, 2022 at 3:12 AM RyanLin <[email protected]> wrote:
>
> [Why]
> External displays take priority over internal display when there are fewer
> display controllers than displays.
>
> [How]
> The root cause is because of that number of the crtc is not correct.
> The number of the crtc on the 3250c is 3, but on the 3500c is 4.
> On the source code, we can see that number of the crtc has been fixed at 4.
> Needs to set the num_crtc to 3 for 3250c platform.
>
> Signed-off-by: RyanLin <[email protected]>
> Change-Id: I837df7101cc4849d2c3021fd529b4061edab4bb1
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 40c91b448f7d..dbeef7b57a9b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
> break;
> #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
> case CHIP_RAVEN:
> - adev->mode_info.num_crtc = 4;
> - adev->mode_info.num_hpd = 4;
> - adev->mode_info.num_dig = 4;
> + if (adev->rev_id >= 8) { //chip_name = "raven2";
> + adev->mode_info.num_crtc = 3;
> + adev->mode_info.num_hpd = 3;
> + adev->mode_info.num_dig = 3;
> + } else {
> + adev->mode_info.num_crtc = 4;
> + adev->mode_info.num_hpd = 4;
> + adev->mode_info.num_dig = 4;
> + }
> break;
> #endif
> #if defined(CONFIG_DRM_AMD_DC_DCN2_0)
> --
> 2.25.1
>
On Thu, Jan 27, 2022 at 3:12 AM RyanLin <[email protected]> wrote:
>
> [Why]
> External displays take priority over internal display when there are fewer
> display controllers than displays.
>
> [How]
> The root cause is because of that number of the crtc is not correct.
> The number of the crtc on the 3250c is 3, but on the 3500c is 4.
> On the source code, we can see that number of the crtc has been fixed at 4.
> Needs to set the num_crtc to 3 for 3250c platform.
>
> Signed-off-by: RyanLin <[email protected]>
> Change-Id: I837df7101cc4849d2c3021fd529b4061edab4bb1
Please drop the gerrit ID.
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 40c91b448f7d..dbeef7b57a9b 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
> break;
> #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
> case CHIP_RAVEN:
> - adev->mode_info.num_crtc = 4;
> - adev->mode_info.num_hpd = 4;
> - adev->mode_info.num_dig = 4;
> + if (adev->rev_id >= 8) { //chip_name = "raven2";
> + adev->mode_info.num_crtc = 3;
> + adev->mode_info.num_hpd = 3;
> + adev->mode_info.num_dig = 3;
> + } else {
> + adev->mode_info.num_crtc = 4;
> + adev->mode_info.num_hpd = 4;
> + adev->mode_info.num_dig = 4;
> + }
> break;
> #endif
> #if defined(CONFIG_DRM_AMD_DC_DCN2_0)
> --
> 2.25.1
>
[Why]
External displays take priority over internal display when there are fewer
display controllers than displays.
[How]
The root cause is because of that number of the crtc is not correct.
The number of the crtc on the 3250c is 3, but on the 3500c is 4.
On the source code, we can see that number of the crtc has been fixed at 4.
Needs to set the num_crtc to 3 for 3250c platform.
v2:
- remove unnecessary comments and Id
Signed-off-by: Ryan Lin <[email protected]>
---
drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 40c91b448f7da..455a2c45e8cda 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
break;
#if defined(CONFIG_DRM_AMD_DC_DCN1_0)
case CHIP_RAVEN:
- adev->mode_info.num_crtc = 4;
- adev->mode_info.num_hpd = 4;
- adev->mode_info.num_dig = 4;
+ if (adev->rev_id >= 8) {
+ adev->mode_info.num_crtc = 3;
+ adev->mode_info.num_hpd = 3;
+ adev->mode_info.num_dig = 3;
+ } else {
+ adev->mode_info.num_crtc = 4;
+ adev->mode_info.num_hpd = 4;
+ adev->mode_info.num_dig = 4;
+ }
break;
#endif
#if defined(CONFIG_DRM_AMD_DC_DCN2_0)
--
2.25.1
[Cc: Remove undeliverable Chun Ming Zhou <mailto:[email protected]>]
Am 30.03.22 um 08:34 schrieb Paul Menzel:
> Dear Tsung-Hua,
>
>
> Thank you for your patch.
>
> Am 30.03.22 um 04:46 schrieb Ryan Lin:
>
> The commit message summary is quite long and confusing. Maybe:
>
> Use 3 CRTC for 3250c to get internal display working
>
>> [Why]
>> External displays take priority over internal display when there are
>> fewer
>> display controllers than displays.
>
> This causes the internal display to not work on the Chromebook google/zork.
>
>> [How]
>> The root cause is because of that number of the crtc is not correct.
>
> The root cause is the incorrect number of four configured CRTCs.
>
>> The number of the crtc on the 3250c is 3, but on the 3500c is 4.
>> On the source code, we can see that number of the crtc has been fixed
>> at 4.
>> Needs to set the num_crtc to 3 for 3250c platform.
>
> Please do not wrap lines after each sentence, and use a text width of 75
> characters.
>
>> v2:
>> - remove unnecessary comments and Id
>>
>> Signed-off-by: Ryan Lin <[email protected]>
>>
>> ---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> index 40c91b448f7da..455a2c45e8cda 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>> @@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
>> break;
>> #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
>> case CHIP_RAVEN:
>> - adev->mode_info.num_crtc = 4;
>> - adev->mode_info.num_hpd = 4;
>> - adev->mode_info.num_dig = 4;
>> + if (adev->rev_id >= 8) {
>
> Is there some define for that number? Maybe add a comment, that it’s for
> 3250c?
>
>
> Kind regards,
>
> Paul
>
>
>> + adev->mode_info.num_crtc = 3;
>> + adev->mode_info.num_hpd = 3;
>> + adev->mode_info.num_dig = 3;
>> + } else {
>> + adev->mode_info.num_crtc = 4;
>> + adev->mode_info.num_hpd = 4;
>> + adev->mode_info.num_dig = 4;
>> + }
>> break;
>> #endif
>> #if defined(CONFIG_DRM_AMD_DC_DCN2_0)
Dear Tsung-Hua,
Thank you for your patch.
Am 30.03.22 um 04:46 schrieb Ryan Lin:
The commit message summary is quite long and confusing. Maybe:
Use 3 CRTC for 3250c to get internal display working
> [Why]
> External displays take priority over internal display when there are fewer
> display controllers than displays.
This causes the internal display to not work on the Chromebook google/zork.
> [How]
> The root cause is because of that number of the crtc is not correct.
The root cause is the incorrect number of four configured CRTCs.
> The number of the crtc on the 3250c is 3, but on the 3500c is 4.
> On the source code, we can see that number of the crtc has been fixed at 4.
> Needs to set the num_crtc to 3 for 3250c platform.
Please do not wrap lines after each sentence, and use a text width of 75
characters.
> v2:
> - remove unnecessary comments and Id
>
> Signed-off-by: Ryan Lin <[email protected]>
>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 40c91b448f7da..455a2c45e8cda 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
> break;
> #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
> case CHIP_RAVEN:
> - adev->mode_info.num_crtc = 4;
> - adev->mode_info.num_hpd = 4;
> - adev->mode_info.num_dig = 4;
> + if (adev->rev_id >= 8) {
Is there some define for that number? Maybe add a comment, that it’s for
3250c?
Kind regards,
Paul
> + adev->mode_info.num_crtc = 3;
> + adev->mode_info.num_hpd = 3;
> + adev->mode_info.num_dig = 3;
> + } else {
> + adev->mode_info.num_crtc = 4;
> + adev->mode_info.num_hpd = 4;
> + adev->mode_info.num_dig = 4;
> + }
> break;
> #endif
> #if defined(CONFIG_DRM_AMD_DC_DCN2_0)
Hi Ryan,
Is this change applicable on a specific kernel version?
On latest I see IP DISCOVERY based impl for CHIP_RAVEN.
>[Why]
>External displays take priority over internal display when there are fewer display controllers than displays.
>
> [How]
>The root cause is because of that number of the crtc is not correct.
>The number of the crtc on the 3250c is 3, but on the 3500c is 4.
>On the source code, we can see that number of the crtc has been fixed at 4.
>Needs to set the num_crtc to 3 for 3250c platform.
>
>v2:
> - remove unnecessary comments and Id
>
>Signed-off-by: Ryan Lin <[email protected]>
>
>---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>index 40c91b448f7da..455a2c45e8cda 100644
>--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>@@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
> break;
> #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
> case CHIP_RAVEN:
>- adev->mode_info.num_crtc = 4;
>- adev->mode_info.num_hpd = 4;
>- adev->mode_info.num_dig = 4;
>+ if (adev->rev_id >= 8) {
May I know what this ">=8" indicate? Also, should it be external_rev_id if its based on old version?
>+ adev->mode_info.num_crtc = 3;
>+ adev->mode_info.num_hpd = 3;
>+ adev->mode_info.num_dig = 3;
>+ } else {
>+ adev->mode_info.num_crtc = 4;
>+ adev->mode_info.num_hpd = 4;
>+ adev->mode_info.num_dig = 4;
>+ }
> break;
> #endif
> #if defined(CONFIG_DRM_AMD_DC_DCN2_0)
>--
>2.25.1
>
BR,
Chandan V N
On Tue, Mar 29, 2022 at 10:57 PM Ryan Lin <[email protected]> wrote:
>
> [Why]
> External displays take priority over internal display when there are fewer
> display controllers than displays.
>
> [How]
> The root cause is because of that number of the crtc is not correct.
> The number of the crtc on the 3250c is 3, but on the 3500c is 4.
> On the source code, we can see that number of the crtc has been fixed at 4.
> Needs to set the num_crtc to 3 for 3250c platform.
>
> v2:
> - remove unnecessary comments and Id
>
> Signed-off-by: Ryan Lin <[email protected]>
Reviewed-by: Alex Deucher <[email protected]>
>
> ---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 40c91b448f7da..455a2c45e8cda 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
> break;
> #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
> case CHIP_RAVEN:
> - adev->mode_info.num_crtc = 4;
> - adev->mode_info.num_hpd = 4;
> - adev->mode_info.num_dig = 4;
> + if (adev->rev_id >= 8) {
> + adev->mode_info.num_crtc = 3;
> + adev->mode_info.num_hpd = 3;
> + adev->mode_info.num_dig = 3;
> + } else {
> + adev->mode_info.num_crtc = 4;
> + adev->mode_info.num_hpd = 4;
> + adev->mode_info.num_dig = 4;
> + }
> break;
> #endif
> #if defined(CONFIG_DRM_AMD_DC_DCN2_0)
> --
> 2.25.1
>
>Hi Chandan,
>
>This issue we found on the Zork project which uses the kernel 5.4 on. So I just implemented it on kernel 5.4.
>For finding out which is 3250c, I referenced the function which is implemented from another function.
>Below is the part where I found it.
>
>drivers/gpu/drm/amd/amdgpu/amdgpu_device.c-
> case CHIP_RAVEN:
> if (adev->rev_id >= 8)
> chip_name = "raven2";
> else if (adev->pdev->device == 0x15d8)
> chip_name = "picasso";
> else
> chip_name = "raven";
> break;
>
>BR,
>Ryan Lin.
Suggest you to rebase to amd-staging-drm-next tip and update your changes accordingly and re-submit.
>
>Hi Ryan,
>
>Is this change applicable on a specific kernel version?
>On latest I see IP DISCOVERY based impl for CHIP_RAVEN.
>
>>[Why]
>>External displays take priority over internal display when there are fewer display controllers than displays.
>>
>> [How]
>>The root cause is because of that number of the crtc is not correct.
>>The number of the crtc on the 3250c is 3, but on the 3500c is 4.
>>On the source code, we can see that number of the crtc has been fixed at 4.
>>Needs to set the num_crtc to 3 for 3250c platform.
>>
>>v2:
>> - remove unnecessary comments and Id
>>
>>Signed-off-by: Ryan Lin <[email protected]>
>>
>>---
>> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
>> 1 file changed, 9 insertions(+), 3 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>index 40c91b448f7da..455a2c45e8cda 100644
>>--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>>@@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
>> break;
>> #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
>> case CHIP_RAVEN:
>>- adev->mode_info.num_crtc = 4;
>>- adev->mode_info.num_hpd = 4;
>>- adev->mode_info.num_dig = 4;
>>+ if (adev->rev_id >= 8) {
>
>May I know what this ">=8" indicate? Also, should it be external_rev_id if its based on old version?
>
>>+ adev->mode_info.num_crtc = 3;
>>+ adev->mode_info.num_hpd = 3;
>>+ adev->mode_info.num_dig = 3;
>>+ } else {
>>+ adev->mode_info.num_crtc = 4;
>>+ adev->mode_info.num_hpd = 4;
>>+ adev->mode_info.num_dig = 4;
>>+ }
>> break;
>> #endif
>> #if defined(CONFIG_DRM_AMD_DC_DCN2_0)
>>--
>>2.25.1
>>
>
>BR,
>Chandan V N
[AMD Official Use Only]
Hi Chandan,
This issue we found on the Zork project which uses the kernel 5.4 on. So I just implemented it on kernel 5.4.
For finding out which is 3250c, I referenced the function which is implemented from another function.
Below is the part where I found it.
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c-
case CHIP_RAVEN:
if (adev->rev_id >= 8)
chip_name = "raven2";
else if (adev->pdev->device == 0x15d8)
chip_name = "picasso";
else
chip_name = "raven";
break;
BR,
Ryan Lin.
-----Original Message-----
From: VURDIGERENATARAJ, CHANDAN <[email protected]>
Sent: Wednesday, March 30, 2022 12:30 PM
To: Lin, Tsung-hua (Ryan) <[email protected]>
Cc: David (ChunMing) Zhou <[email protected]>; Drew Davenport <[email protected]>; Li, Sun peng (Leo) <[email protected]>; Li, Leon <[email protected]>; [email protected]; Siqueira, Rodrigo <[email protected]>; [email protected]; [email protected]; Koenig, Christian <[email protected]>; David Airlie <[email protected]>; Sean Paul <[email protected]>; Louis Li <[email protected]>; Daniel Vetter <[email protected]>; Bas Nieuwenhuizen <[email protected]>; Deucher, Alexander <[email protected]>; St?phane Marchesin <[email protected]>; Kazlauskas, Nicholas <[email protected]>; Wentland, Harry <[email protected]>; Lin, Tsung-hua (Ryan) <[email protected]>; Mark Yacoub <[email protected]>
Subject: RE: [PATCH v2] drm/amdgpu: fix that issue that the number of the crtc of the 3250c is not correct
Hi Ryan,
Is this change applicable on a specific kernel version?
On latest I see IP DISCOVERY based impl for CHIP_RAVEN.
>[Why]
>External displays take priority over internal display when there are fewer display controllers than displays.
>
> [How]
>The root cause is because of that number of the crtc is not correct.
>The number of the crtc on the 3250c is 3, but on the 3500c is 4.
>On the source code, we can see that number of the crtc has been fixed at 4.
>Needs to set the num_crtc to 3 for 3250c platform.
>
>v2:
> - remove unnecessary comments and Id
>
>Signed-off-by: Ryan Lin <[email protected]>
>
>---
> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>index 40c91b448f7da..455a2c45e8cda 100644
>--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
>@@ -2738,9 +2738,15 @@ static int dm_early_init(void *handle)
> break;
> #if defined(CONFIG_DRM_AMD_DC_DCN1_0)
> case CHIP_RAVEN:
>- adev->mode_info.num_crtc = 4;
>- adev->mode_info.num_hpd = 4;
>- adev->mode_info.num_dig = 4;
>+ if (adev->rev_id >= 8) {
May I know what this ">=8" indicate? Also, should it be external_rev_id if its based on old version?
>+ adev->mode_info.num_crtc = 3;
>+ adev->mode_info.num_hpd = 3;
>+ adev->mode_info.num_dig = 3;
>+ } else {
>+ adev->mode_info.num_crtc = 4;
>+ adev->mode_info.num_hpd = 4;
>+ adev->mode_info.num_dig = 4;
>+ }
> break;
> #endif
> #if defined(CONFIG_DRM_AMD_DC_DCN2_0)
>--
>2.25.1
>
BR,
Chandan V N