diff mbox

drm/radeon: avoid turning off spread spectrum for used pll

Message ID 1345228804-3322-1-git-send-email-j.glisse@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jerome Glisse Aug. 17, 2012, 6:40 p.m. UTC
From: Jerome Glisse <jglisse@redhat.com>

If spread spectrum is enabled and in use for a given pll we
should not turn it off as it will lead to turning off display
for crtc that use the pll (this behavior was observed on chelsea
edp).

Signed-off-by: Jerome Glisse <jglisse@redhat.com>
---
 drivers/gpu/drm/radeon/atombios_crtc.c |   25 +++++++++++++++++++++----
 1 files changed, 21 insertions(+), 4 deletions(-)

Comments

Alex Deucher Aug. 17, 2012, 6:54 p.m. UTC | #1
On Fri, Aug 17, 2012 at 2:40 PM,  <j.glisse@gmail.com> wrote:
> From: Jerome Glisse <jglisse@redhat.com>
>
> If spread spectrum is enabled and in use for a given pll we
> should not turn it off as it will lead to turning off display
> for crtc that use the pll (this behavior was observed on chelsea
> edp).
>
> Signed-off-by: Jerome Glisse <jglisse@redhat.com>

Looks good.  I think an alternative would be to just always enable SS
for DP/eDP rather than checking the DPCD since all DP connections run
on the same PLL.  IIRC, that's what the closed driver does.

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/radeon/atombios_crtc.c |   25 +++++++++++++++++++++----
>  1 files changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
> index c6fcb5b..cb18813 100644
> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
> @@ -444,11 +444,28 @@ union atom_enable_ss {
>  static void atombios_crtc_program_ss(struct radeon_device *rdev,
>                                      int enable,
>                                      int pll_id,
> +                                    int crtc_id,
>                                      struct radeon_atom_ss *ss)
>  {
> +       unsigned i;
>         int index = GetIndexIntoMasterTable(COMMAND, EnableSpreadSpectrumOnPPLL);
>         union atom_enable_ss args;
>
> +       if (!enable) {
> +               for (i = 0; i < 6; i++) {
> +                       if (rdev->mode_info.crtcs[i] &&
> +                           rdev->mode_info.crtcs[i]->enabled &&
> +                           i != crtc_id &&
> +                           pll_id == rdev->mode_info.crtcs[i]->pll_id) {
> +                               /* one other crtc is using this pll don't turn
> +                                * off spread spectrum as it might turn off
> +                                * display on active crtc
> +                                */
> +                               return;
> +                       }
> +               }
> +       }
> +
>         memset(&args, 0, sizeof(args));
>
>         if (ASIC_IS_DCE5(rdev)) {
> @@ -1028,7 +1045,7 @@ static void atombios_crtc_set_pll(struct drm_crtc *crtc, struct drm_display_mode
>                 radeon_compute_pll_legacy(pll, adjusted_clock, &pll_clock, &fb_div, &frac_fb_div,
>                                           &ref_div, &post_div);
>
> -       atombios_crtc_program_ss(rdev, ATOM_DISABLE, radeon_crtc->pll_id, &ss);
> +       atombios_crtc_program_ss(rdev, ATOM_DISABLE, radeon_crtc->pll_id, radeon_crtc->crtc_id, &ss);
>
>         atombios_crtc_program_pll(crtc, radeon_crtc->crtc_id, radeon_crtc->pll_id,
>                                   encoder_mode, radeon_encoder->encoder_id, mode->clock,
> @@ -1051,7 +1068,7 @@ static void atombios_crtc_set_pll(struct drm_crtc *crtc, struct drm_display_mode
>                         ss.step = step_size;
>                 }
>
> -               atombios_crtc_program_ss(rdev, ATOM_ENABLE, radeon_crtc->pll_id, &ss);
> +               atombios_crtc_program_ss(rdev, ATOM_ENABLE, radeon_crtc->pll_id, radeon_crtc->crtc_id, &ss);
>         }
>  }
>
> @@ -1572,11 +1589,11 @@ void radeon_atom_disp_eng_pll_init(struct radeon_device *rdev)
>                                                                    ASIC_INTERNAL_SS_ON_DCPLL,
>                                                                    rdev->clock.default_dispclk);
>                 if (ss_enabled)
> -                       atombios_crtc_program_ss(rdev, ATOM_DISABLE, ATOM_DCPLL, &ss);
> +                       atombios_crtc_program_ss(rdev, ATOM_DISABLE, ATOM_DCPLL, -1, &ss);
>                 /* XXX: DCE5, make sure voltage, dispclk is high enough */
>                 atombios_crtc_set_disp_eng_pll(rdev, rdev->clock.default_dispclk);
>                 if (ss_enabled)
> -                       atombios_crtc_program_ss(rdev, ATOM_ENABLE, ATOM_DCPLL, &ss);
> +                       atombios_crtc_program_ss(rdev, ATOM_ENABLE, ATOM_DCPLL, -1, &ss);
>         }
>
>  }
> --
> 1.7.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
Alex Deucher Aug. 17, 2012, 6:55 p.m. UTC | #2
On Fri, Aug 17, 2012 at 2:54 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Fri, Aug 17, 2012 at 2:40 PM,  <j.glisse@gmail.com> wrote:
>> From: Jerome Glisse <jglisse@redhat.com>
>>
>> If spread spectrum is enabled and in use for a given pll we
>> should not turn it off as it will lead to turning off display
>> for crtc that use the pll (this behavior was observed on chelsea
>> edp).
>>
>> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
>
> Looks good.  I think an alternative would be to just always enable SS
> for DP/eDP rather than checking the DPCD since all DP connections run
> on the same PLL.

On discrete cards anyway.  APUs are a little different, but the same idea holds.

Alex
Jerome Glisse Aug. 17, 2012, 10:07 p.m. UTC | #3
On Fri, Aug 17, 2012 at 2:54 PM, Alex Deucher <alexdeucher@gmail.com> wrote:
> On Fri, Aug 17, 2012 at 2:40 PM,  <j.glisse@gmail.com> wrote:
>> From: Jerome Glisse <jglisse@redhat.com>
>>
>> If spread spectrum is enabled and in use for a given pll we
>> should not turn it off as it will lead to turning off display
>> for crtc that use the pll (this behavior was observed on chelsea
>> edp).
>>
>> Signed-off-by: Jerome Glisse <jglisse@redhat.com>
>
> Looks good.  I think an alternative would be to just always enable SS
> for DP/eDP rather than checking the DPCD since all DP connections run
> on the same PLL.  IIRC, that's what the closed driver does.
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

Forgot cc stable

>
>> ---
>>  drivers/gpu/drm/radeon/atombios_crtc.c |   25 +++++++++++++++++++++----
>>  1 files changed, 21 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
>> index c6fcb5b..cb18813 100644
>> --- a/drivers/gpu/drm/radeon/atombios_crtc.c
>> +++ b/drivers/gpu/drm/radeon/atombios_crtc.c
>> @@ -444,11 +444,28 @@ union atom_enable_ss {
>>  static void atombios_crtc_program_ss(struct radeon_device *rdev,
>>                                      int enable,
>>                                      int pll_id,
>> +                                    int crtc_id,
>>                                      struct radeon_atom_ss *ss)
>>  {
>> +       unsigned i;
>>         int index = GetIndexIntoMasterTable(COMMAND, EnableSpreadSpectrumOnPPLL);
>>         union atom_enable_ss args;
>>
>> +       if (!enable) {
>> +               for (i = 0; i < 6; i++) {
>> +                       if (rdev->mode_info.crtcs[i] &&
>> +                           rdev->mode_info.crtcs[i]->enabled &&
>> +                           i != crtc_id &&
>> +                           pll_id == rdev->mode_info.crtcs[i]->pll_id) {
>> +                               /* one other crtc is using this pll don't turn
>> +                                * off spread spectrum as it might turn off
>> +                                * display on active crtc
>> +                                */
>> +                               return;
>> +                       }
>> +               }
>> +       }
>> +
>>         memset(&args, 0, sizeof(args));
>>
>>         if (ASIC_IS_DCE5(rdev)) {
>> @@ -1028,7 +1045,7 @@ static void atombios_crtc_set_pll(struct drm_crtc *crtc, struct drm_display_mode
>>                 radeon_compute_pll_legacy(pll, adjusted_clock, &pll_clock, &fb_div, &frac_fb_div,
>>                                           &ref_div, &post_div);
>>
>> -       atombios_crtc_program_ss(rdev, ATOM_DISABLE, radeon_crtc->pll_id, &ss);
>> +       atombios_crtc_program_ss(rdev, ATOM_DISABLE, radeon_crtc->pll_id, radeon_crtc->crtc_id, &ss);
>>
>>         atombios_crtc_program_pll(crtc, radeon_crtc->crtc_id, radeon_crtc->pll_id,
>>                                   encoder_mode, radeon_encoder->encoder_id, mode->clock,
>> @@ -1051,7 +1068,7 @@ static void atombios_crtc_set_pll(struct drm_crtc *crtc, struct drm_display_mode
>>                         ss.step = step_size;
>>                 }
>>
>> -               atombios_crtc_program_ss(rdev, ATOM_ENABLE, radeon_crtc->pll_id, &ss);
>> +               atombios_crtc_program_ss(rdev, ATOM_ENABLE, radeon_crtc->pll_id, radeon_crtc->crtc_id, &ss);
>>         }
>>  }
>>
>> @@ -1572,11 +1589,11 @@ void radeon_atom_disp_eng_pll_init(struct radeon_device *rdev)
>>                                                                    ASIC_INTERNAL_SS_ON_DCPLL,
>>                                                                    rdev->clock.default_dispclk);
>>                 if (ss_enabled)
>> -                       atombios_crtc_program_ss(rdev, ATOM_DISABLE, ATOM_DCPLL, &ss);
>> +                       atombios_crtc_program_ss(rdev, ATOM_DISABLE, ATOM_DCPLL, -1, &ss);
>>                 /* XXX: DCE5, make sure voltage, dispclk is high enough */
>>                 atombios_crtc_set_disp_eng_pll(rdev, rdev->clock.default_dispclk);
>>                 if (ss_enabled)
>> -                       atombios_crtc_program_ss(rdev, ATOM_ENABLE, ATOM_DCPLL, &ss);
>> +                       atombios_crtc_program_ss(rdev, ATOM_ENABLE, ATOM_DCPLL, -1, &ss);
>>         }
>>
>>  }
>> --
>> 1.7.1
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index c6fcb5b..cb18813 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -444,11 +444,28 @@  union atom_enable_ss {
 static void atombios_crtc_program_ss(struct radeon_device *rdev,
 				     int enable,
 				     int pll_id,
+				     int crtc_id,
 				     struct radeon_atom_ss *ss)
 {
+	unsigned i;
 	int index = GetIndexIntoMasterTable(COMMAND, EnableSpreadSpectrumOnPPLL);
 	union atom_enable_ss args;
 
+	if (!enable) {
+		for (i = 0; i < 6; i++) {
+			if (rdev->mode_info.crtcs[i] &&
+			    rdev->mode_info.crtcs[i]->enabled &&
+			    i != crtc_id &&
+			    pll_id == rdev->mode_info.crtcs[i]->pll_id) {
+				/* one other crtc is using this pll don't turn
+				 * off spread spectrum as it might turn off
+				 * display on active crtc
+				 */
+				return;
+			}
+		}
+	}
+
 	memset(&args, 0, sizeof(args));
 
 	if (ASIC_IS_DCE5(rdev)) {
@@ -1028,7 +1045,7 @@  static void atombios_crtc_set_pll(struct drm_crtc *crtc, struct drm_display_mode
 		radeon_compute_pll_legacy(pll, adjusted_clock, &pll_clock, &fb_div, &frac_fb_div,
 					  &ref_div, &post_div);
 
-	atombios_crtc_program_ss(rdev, ATOM_DISABLE, radeon_crtc->pll_id, &ss);
+	atombios_crtc_program_ss(rdev, ATOM_DISABLE, radeon_crtc->pll_id, radeon_crtc->crtc_id, &ss);
 
 	atombios_crtc_program_pll(crtc, radeon_crtc->crtc_id, radeon_crtc->pll_id,
 				  encoder_mode, radeon_encoder->encoder_id, mode->clock,
@@ -1051,7 +1068,7 @@  static void atombios_crtc_set_pll(struct drm_crtc *crtc, struct drm_display_mode
 			ss.step = step_size;
 		}
 
-		atombios_crtc_program_ss(rdev, ATOM_ENABLE, radeon_crtc->pll_id, &ss);
+		atombios_crtc_program_ss(rdev, ATOM_ENABLE, radeon_crtc->pll_id, radeon_crtc->crtc_id, &ss);
 	}
 }
 
@@ -1572,11 +1589,11 @@  void radeon_atom_disp_eng_pll_init(struct radeon_device *rdev)
 								   ASIC_INTERNAL_SS_ON_DCPLL,
 								   rdev->clock.default_dispclk);
 		if (ss_enabled)
-			atombios_crtc_program_ss(rdev, ATOM_DISABLE, ATOM_DCPLL, &ss);
+			atombios_crtc_program_ss(rdev, ATOM_DISABLE, ATOM_DCPLL, -1, &ss);
 		/* XXX: DCE5, make sure voltage, dispclk is high enough */
 		atombios_crtc_set_disp_eng_pll(rdev, rdev->clock.default_dispclk);
 		if (ss_enabled)
-			atombios_crtc_program_ss(rdev, ATOM_ENABLE, ATOM_DCPLL, &ss);
+			atombios_crtc_program_ss(rdev, ATOM_ENABLE, ATOM_DCPLL, -1, &ss);
 	}
 
 }