Re: [PATCH] drm/msm/dp: force link training for display resolution change

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 5/26/2022 5:07 PM, Stephen Boyd wrote:
Quoting Kuogee Hsieh (2022-05-26 10:26:18)
During display resolution changes display have to be disabled first
followed by display enable with new resolution. This patch force
main link always be retrained during display enable procedure to
simplify implementation instead of manually kicking of irq_hpd
handle.
How is that better? Can you please describe how it improves things? I
suppose it avoids some round trips through the event queue just to turn
on the display?

This try to fix customer issues which only happen on one particular panel which fails on display resolution changes sometimes.

I will add more description.


Signed-off-by: Kuogee Hsieh <quic_khsieh@xxxxxxxxxxx>
Any Fixes tag? Or it's not fixing anything, just making things simpler?
will add.

diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.c b/drivers/gpu/drm/msm/dp/dp_ctrl.c
index 5ddb4e8..8b71013 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.c
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.c
diff --git a/drivers/gpu/drm/msm/dp/dp_ctrl.h b/drivers/gpu/drm/msm/dp/dp_ctrl.h
index 2433edb..dcc7af21 100644
--- a/drivers/gpu/drm/msm/dp/dp_ctrl.h
+++ b/drivers/gpu/drm/msm/dp/dp_ctrl.h
@@ -20,7 +20,7 @@ struct dp_ctrl {
  };

  int dp_ctrl_on_link(struct dp_ctrl *dp_ctrl);
-int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl);
+int dp_ctrl_on_stream(struct dp_ctrl *dp_ctrl, bool force_link_train);
Can we have another API like dp_ctrl_on_stream_retrain()? Or name
'force_link_train' to 'retrain'?

  int dp_ctrl_off_link_stream(struct dp_ctrl *dp_ctrl);
  int dp_ctrl_off(struct dp_ctrl *dp_ctrl);
  void dp_ctrl_push_idle(struct dp_ctrl *dp_ctrl);
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index bfc6581..9246421 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -892,7 +892,7 @@ static int dp_display_enable(struct dp_display_private *dp, u32 data)
Any reason why dp_display_enable() is forward declared and why it takes
a u32 data argument? It's not part of the eventqueue calling code from
what I can tell so we should be able to rename 'data' to 'retrain_link'?

                 return 0;
         }

-       rc = dp_ctrl_on_stream(dp->ctrl);
+       rc = dp_ctrl_on_stream(dp->ctrl, data);
         if (!rc)
                 dp_display->power_on = true;

@@ -1594,6 +1594,7 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)
         int rc = 0;
         struct dp_display_private *dp_display;
         u32 state;
+       bool force_link_train = false;

         dp_display = container_of(dp, struct dp_display_private, dp_display);
         if (!dp_display->dp_mode.drm_mode.clock) {
@@ -1622,10 +1623,12 @@ int msm_dp_display_enable(struct msm_dp *dp, struct drm_encoder *encoder)

         state =  dp_display->hpd_state;

-       if (state == ST_DISPLAY_OFF)
+       if (state == ST_DISPLAY_OFF) {
                 dp_display_host_phy_init(dp_display);
+               force_link_train = 1;
Use true instead of 1 if the type is a bool please.

+       }

-       dp_display_enable(dp_display, 0);
+       dp_display_enable(dp_display, force_link_train);

         rc = dp_display_post_enable(dp);
         if (rc) {



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux