diff mbox series

drm/i915/cx0: Only clear/set the Pipe Reset bit of the PHY Lanes Owned

Message ID 20231005001310.154396-1-khaled.almahallawy@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/cx0: Only clear/set the Pipe Reset bit of the PHY Lanes Owned | expand

Commit Message

Almahallawy, Khaled Oct. 5, 2023, 12:13 a.m. UTC
Currently, with MFD/pin assignment D, the driver clears the pipe reset bit
of lane 1 which is not owned by display. This causes the display
to block S0iX.

By not clearing this bit for lane 1 and keeping whatever default, S0ix
started to work. This is already what the driver does at the end
of the phy lane reset sequence (Step#8)

Bspec: 65451

Cc: Mika Kahola <mika.kahola@intel.com>
Cc: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Comments

Gustavo Sousa Oct. 5, 2023, 12:39 p.m. UTC | #1
Quoting Khaled Almahallawy (2023-10-04 21:13:10-03:00)
>Currently, with MFD/pin assignment D, the driver clears the pipe reset bit
>of lane 1 which is not owned by display. This causes the display
>to block S0iX.
>
>By not clearing this bit for lane 1 and keeping whatever default, S0ix
>started to work. This is already what the driver does at the end
>of the phy lane reset sequence (Step#8)
>
>Bspec: 65451
>

We should not have blank lines in the trailers section. This could be fixed
while applying.

>Cc: Mika Kahola <mika.kahola@intel.com>
>Cc: Gustavo Sousa <gustavo.sousa@intel.com>
>Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>

Nice fix. Thanks!

Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>

>---
> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>index abd607b564f1..f653b83a7d4f 100644
>--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>@@ -2596,8 +2596,7 @@ static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915,
>                 drm_warn(&i915->drm, "PHY %c failed to bring out of SOC reset after %dus.\n",
>                          phy_name(phy), XELPDP_PORT_BUF_SOC_READY_TIMEOUT_US);
> 
>-        intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port),
>-                     XELPDP_LANE_PIPE_RESET(0) | XELPDP_LANE_PIPE_RESET(1),
>+        intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port), lane_pipe_reset,
>                      lane_pipe_reset);
> 
>         if (__intel_de_wait_for_register(i915, XELPDP_PORT_BUF_CTL2(port),
>-- 
>2.25.1
>
Jani Nikula Oct. 5, 2023, 1:16 p.m. UTC | #2
On Thu, 05 Oct 2023, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Quoting Khaled Almahallawy (2023-10-04 21:13:10-03:00)
>>Currently, with MFD/pin assignment D, the driver clears the pipe reset bit
>>of lane 1 which is not owned by display. This causes the display
>>to block S0iX.
>>
>>By not clearing this bit for lane 1 and keeping whatever default, S0ix
>>started to work. This is already what the driver does at the end
>>of the phy lane reset sequence (Step#8)
>>
>>Bspec: 65451
>>
>
> We should not have blank lines in the trailers section. This could be fixed
> while applying.

Agreed.

>
>>Cc: Mika Kahola <mika.kahola@intel.com>
>>Cc: Gustavo Sousa <gustavo.sousa@intel.com>
>>Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
>
> Nice fix. Thanks!
>
> Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
>
>>---
>> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>>index abd607b564f1..f653b83a7d4f 100644
>>--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>>+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
>>@@ -2596,8 +2596,7 @@ static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915,
>>                 drm_warn(&i915->drm, "PHY %c failed to bring out of SOC reset after %dus.\n",
>>                          phy_name(phy), XELPDP_PORT_BUF_SOC_READY_TIMEOUT_US);
>> 
>>-        intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port),
>>-                     XELPDP_LANE_PIPE_RESET(0) | XELPDP_LANE_PIPE_RESET(1),
>>+        intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port), lane_pipe_reset,
>>                      lane_pipe_reset);
>> 
>>         if (__intel_de_wait_for_register(i915, XELPDP_PORT_BUF_CTL2(port),
>>-- 
>>2.25.1
>>
Radhakrishna Sripada Oct. 11, 2023, 8:32 p.m. UTC | #3
Fixes: 619a06dba6fa ("drm/i915/mtl: Reset only one lane in case of MFD")

With that pushed the patch with mentioned nits. Thank you for the patch and review.

-- Radhakrishna Sripada

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Thursday, October 5, 2023 6:47 PM
> To: Sousa, Gustavo <gustavo.sousa@intel.com>; Almahallawy, Khaled
> <khaled.almahallawy@intel.com>; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/cx0: Only clear/set the Pipe Reset bit of
> the PHY Lanes Owned
> 
> On Thu, 05 Oct 2023, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> > Quoting Khaled Almahallawy (2023-10-04 21:13:10-03:00)
> >>Currently, with MFD/pin assignment D, the driver clears the pipe reset bit
> >>of lane 1 which is not owned by display. This causes the display
> >>to block S0iX.
> >>
> >>By not clearing this bit for lane 1 and keeping whatever default, S0ix
> >>started to work. This is already what the driver does at the end
> >>of the phy lane reset sequence (Step#8)
> >>
> >>Bspec: 65451
> >>
> >
> > We should not have blank lines in the trailers section. This could be fixed
> > while applying.
> 
> Agreed.
> 
> >
> >>Cc: Mika Kahola <mika.kahola@intel.com>
> >>Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> >>Signed-off-by: Khaled Almahallawy <khaled.almahallawy@intel.com>
> >
> > Nice fix. Thanks!
> >
> > Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
> >
> >>---
> >> drivers/gpu/drm/i915/display/intel_cx0_phy.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >>index abd607b564f1..f653b83a7d4f 100644
> >>--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >>+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
> >>@@ -2596,8 +2596,7 @@ static void intel_cx0_phy_lane_reset(struct
> drm_i915_private *i915,
> >>                 drm_warn(&i915->drm, "PHY %c failed to bring out of SOC reset after
> %dus.\n",
> >>                          phy_name(phy),
> XELPDP_PORT_BUF_SOC_READY_TIMEOUT_US);
> >>
> >>-        intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port),
> >>-                     XELPDP_LANE_PIPE_RESET(0) | XELPDP_LANE_PIPE_RESET(1),
> >>+        intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port), lane_pipe_reset,
> >>                      lane_pipe_reset);
> >>
> >>         if (__intel_de_wait_for_register(i915, XELPDP_PORT_BUF_CTL2(port),
> >>--
> >>2.25.1
> >>
> 
> --
> Jani Nikula, Intel
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_cx0_phy.c b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
index abd607b564f1..f653b83a7d4f 100644
--- a/drivers/gpu/drm/i915/display/intel_cx0_phy.c
+++ b/drivers/gpu/drm/i915/display/intel_cx0_phy.c
@@ -2596,8 +2596,7 @@  static void intel_cx0_phy_lane_reset(struct drm_i915_private *i915,
 		drm_warn(&i915->drm, "PHY %c failed to bring out of SOC reset after %dus.\n",
 			 phy_name(phy), XELPDP_PORT_BUF_SOC_READY_TIMEOUT_US);
 
-	intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port),
-		     XELPDP_LANE_PIPE_RESET(0) | XELPDP_LANE_PIPE_RESET(1),
+	intel_de_rmw(i915, XELPDP_PORT_BUF_CTL2(port), lane_pipe_reset,
 		     lane_pipe_reset);
 
 	if (__intel_de_wait_for_register(i915, XELPDP_PORT_BUF_CTL2(port),