| Message ID | 20230419-dpu-tweaks-v1-3-d1bac46db075@freebox.fr (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | drm/msm/dpu: tweaks for better hardware resources allocation | expand |
On 19/04/2023 17:41, Arnaud Vrac wrote: > Do not override the hsync/vsync polarity passed by the encoder when > setting up intf timings. The same logic was used in both the encoder and > intf code to set the DP and DSI polarities, so those interfaces are not > impacted. However for HDMI, the polarities were overriden to static > values based on the vertical resolution, instead of using the actual > mode polarities. > > Signed-off-by: Arnaud Vrac <avrac@freebox.fr> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> As a side note: I think at some point we should get rid of the override in dpu_encoder too and move it to dsi bridge code.
On 4/19/2023 7:41 AM, Arnaud Vrac wrote: > Do not override the hsync/vsync polarity passed by the encoder when > setting up intf timings. The same logic was used in both the encoder and > intf code to set the DP and DSI polarities, so those interfaces are not > impacted. However for HDMI, the polarities were overriden to static > values based on the vertical resolution, instead of using the actual > mode polarities. > Any idea why vres based static polarity override was in place? Hope you had tested HDMI resolutions with yres > and < than 720. Jeykumar S. > Signed-off-by: Arnaud Vrac <avrac@freebox.fr> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 16 +++------------- > 1 file changed, 3 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > index 84ee2efa9c664..9f05417eb1213 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > @@ -104,7 +104,7 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx, > u32 active_h_start, active_h_end; > u32 active_v_start, active_v_end; > u32 active_hctl, display_hctl, hsync_ctl; > - u32 polarity_ctl, den_polarity, hsync_polarity, vsync_polarity; > + u32 polarity_ctl, den_polarity; > u32 panel_format; > u32 intf_cfg, intf_cfg2 = 0; > u32 display_data_hctl = 0, active_data_hctl = 0; > @@ -191,19 +191,9 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx, > } > > den_polarity = 0; > - if (ctx->cap->type == INTF_HDMI) { > - hsync_polarity = p->yres >= 720 ? 0 : 1; > - vsync_polarity = p->yres >= 720 ? 0 : 1; > - } else if (ctx->cap->type == INTF_DP) { > - hsync_polarity = p->hsync_polarity; > - vsync_polarity = p->vsync_polarity; > - } else { > - hsync_polarity = 0; > - vsync_polarity = 0; > - } > polarity_ctl = (den_polarity << 2) | /* DEN Polarity */ > - (vsync_polarity << 1) | /* VSYNC Polarity */ > - (hsync_polarity << 0); /* HSYNC Polarity */ > + (p->vsync_polarity << 1) | /* VSYNC Polarity */ > + (p->hsync_polarity << 0); /* HSYNC Polarity */ > > if (!DPU_FORMAT_IS_YUV(fmt)) > panel_format = (fmt->bits[C0_G_Y] | >
On Thu, 20 Apr 2023 at 21:01, Jeykumar Sankaran <quic_jeykumar@quicinc.com> wrote: > > > > On 4/19/2023 7:41 AM, Arnaud Vrac wrote: > > Do not override the hsync/vsync polarity passed by the encoder when > > setting up intf timings. The same logic was used in both the encoder and > > intf code to set the DP and DSI polarities, so those interfaces are not > > impacted. However for HDMI, the polarities were overriden to static > > values based on the vertical resolution, instead of using the actual > > mode polarities. > > > Any idea why vres based static polarity override was in place? Hope you > had tested HDMI resolutions with yres > and < than 720. I think it migrated from the old fbdev, see [1]. Tthis easily causes issues with the DVI monitors, which have different rules for hsync/vsync. I have observed this on other platforms. In practice it was observed that EDID provides correct sync polarity, so it is better to follow the EDID data. If for some reason EDID is incorrect, DRM provides a way to override it. [1] https://git.codelinaro.org/clo/la/kernel/msm-3.18/-/commit/29c3d616bf3f89f13bbab46c58151ae7a3a79b98 > > Jeykumar S. > > > Signed-off-by: Arnaud Vrac <avrac@freebox.fr> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 16 +++------------- > > 1 file changed, 3 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > > index 84ee2efa9c664..9f05417eb1213 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > > @@ -104,7 +104,7 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx, > > u32 active_h_start, active_h_end; > > u32 active_v_start, active_v_end; > > u32 active_hctl, display_hctl, hsync_ctl; > > - u32 polarity_ctl, den_polarity, hsync_polarity, vsync_polarity; > > + u32 polarity_ctl, den_polarity; > > u32 panel_format; > > u32 intf_cfg, intf_cfg2 = 0; > > u32 display_data_hctl = 0, active_data_hctl = 0; > > @@ -191,19 +191,9 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx, > > } > > > > den_polarity = 0; > > - if (ctx->cap->type == INTF_HDMI) { > > - hsync_polarity = p->yres >= 720 ? 0 : 1; > > - vsync_polarity = p->yres >= 720 ? 0 : 1; > > - } else if (ctx->cap->type == INTF_DP) { > > - hsync_polarity = p->hsync_polarity; > > - vsync_polarity = p->vsync_polarity; > > - } else { > > - hsync_polarity = 0; > > - vsync_polarity = 0; > > - } > > polarity_ctl = (den_polarity << 2) | /* DEN Polarity */ > > - (vsync_polarity << 1) | /* VSYNC Polarity */ > > - (hsync_polarity << 0); /* HSYNC Polarity */ > > + (p->vsync_polarity << 1) | /* VSYNC Polarity */ > > + (p->hsync_polarity << 0); /* HSYNC Polarity */ > > > > if (!DPU_FORMAT_IS_YUV(fmt)) > > panel_format = (fmt->bits[C0_G_Y] | > >
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c index 84ee2efa9c664..9f05417eb1213 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -104,7 +104,7 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx, u32 active_h_start, active_h_end; u32 active_v_start, active_v_end; u32 active_hctl, display_hctl, hsync_ctl; - u32 polarity_ctl, den_polarity, hsync_polarity, vsync_polarity; + u32 polarity_ctl, den_polarity; u32 panel_format; u32 intf_cfg, intf_cfg2 = 0; u32 display_data_hctl = 0, active_data_hctl = 0; @@ -191,19 +191,9 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *ctx, } den_polarity = 0; - if (ctx->cap->type == INTF_HDMI) { - hsync_polarity = p->yres >= 720 ? 0 : 1; - vsync_polarity = p->yres >= 720 ? 0 : 1; - } else if (ctx->cap->type == INTF_DP) { - hsync_polarity = p->hsync_polarity; - vsync_polarity = p->vsync_polarity; - } else { - hsync_polarity = 0; - vsync_polarity = 0; - } polarity_ctl = (den_polarity << 2) | /* DEN Polarity */ - (vsync_polarity << 1) | /* VSYNC Polarity */ - (hsync_polarity << 0); /* HSYNC Polarity */ + (p->vsync_polarity << 1) | /* VSYNC Polarity */ + (p->hsync_polarity << 0); /* HSYNC Polarity */ if (!DPU_FORMAT_IS_YUV(fmt)) panel_format = (fmt->bits[C0_G_Y] |
Do not override the hsync/vsync polarity passed by the encoder when setting up intf timings. The same logic was used in both the encoder and intf code to set the DP and DSI polarities, so those interfaces are not impacted. However for HDMI, the polarities were overriden to static values based on the vertical resolution, instead of using the actual mode polarities. Signed-off-by: Arnaud Vrac <avrac@freebox.fr> --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-)