| Message ID | 20240527-msm-drm-dsc-dsi-video-upstream-4-v5-2-f797ffba4682@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Add DSC support to DSI video panel | expand |
On Mon, May 27, 2024 at 10:21:48PM +0800, Jun Nie wrote: > data is valid for only half the active window if widebus > is enabled > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > 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 225c1c7768ff..f97221423249 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > @@ -168,6 +168,15 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *intf, > > data_width = p->width; > > + /* > + * If widebus is enabled, data is valid for only half the active window > + * since the data rate is doubled in this mode. But for the compression > + * mode in DP case, the p->width is already adjusted in > + * drm_mode_to_intf_timing_params() Is there any reason for divergence here? > + */ > + if (p->wide_bus_en && !dp_intf) > + data_width = p->width >> 1; > + > hsync_data_start_x = hsync_start_x; > hsync_data_end_x = hsync_start_x + data_width - 1; > > > -- > 2.34.1 >
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年5月28日周二 08:48写道： > > On Mon, May 27, 2024 at 10:21:48PM +0800, Jun Nie wrote: > > data is valid for only half the active window if widebus > > is enabled > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > --- > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > 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 225c1c7768ff..f97221423249 100644 > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > > @@ -168,6 +168,15 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *intf, > > > > data_width = p->width; > > > > + /* > > + * If widebus is enabled, data is valid for only half the active window > > + * since the data rate is doubled in this mode. But for the compression > > + * mode in DP case, the p->width is already adjusted in > > + * drm_mode_to_intf_timing_params() > > Is there any reason for divergence here? Lots of parameters in dpu_hw_intf_setup_timing_engine() is calculated from timing->width, such as hsync_period and display_v_start. So the width cannot be adjusted beforehand in drm_mode_to_intf_timing_params(). Otherwise, we get below error. I guess the interface timing configuration differ in silicon, thus the software shall handle the difference. If we adjust the width beforehand for DSI, we get below error. [ 6.625446] [drm:dpu_encoder_frame_done_timeout:2469] [dpu error]enc31 frame done timeout [ 6.642369] [drm:dpu_encoder_phys_vid_wait_for_commit_done:525] [dpu error]vblank timeout: 4200c1 [ 6.642395] [drm:dpu_kms_wait_for_commit_done:493] [dpu error]wait for commit done returned -110 > > > + */ > > + if (p->wide_bus_en && !dp_intf) > > + data_width = p->width >> 1; > > + > > hsync_data_start_x = hsync_start_x; > > hsync_data_end_x = hsync_start_x + data_width - 1; > > > > > > -- > > 2.34.1 > > > > -- > With best wishes > Dmitry
On Tue, May 28, 2024 at 05:59:13PM +0800, Jun Nie wrote: > Dmitry Baryshkov <dmitry.baryshkov@linaro.org> 于2024年5月28日周二 08:48写道： > > > > On Mon, May 27, 2024 at 10:21:48PM +0800, Jun Nie wrote: > > > data is valid for only half the active window if widebus > > > is enabled > > > > > > Signed-off-by: Jun Nie <jun.nie@linaro.org> > > > --- > > > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > 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 225c1c7768ff..f97221423249 100644 > > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > > > @@ -168,6 +168,15 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *intf, > > > > > > data_width = p->width; > > > > > > + /* > > > + * If widebus is enabled, data is valid for only half the active window > > > + * since the data rate is doubled in this mode. But for the compression > > > + * mode in DP case, the p->width is already adjusted in > > > + * drm_mode_to_intf_timing_params() > > > > Is there any reason for divergence here? > > Lots of parameters in dpu_hw_intf_setup_timing_engine() is calculated > from timing->width, > such as hsync_period and display_v_start. So the width cannot be > adjusted beforehand in > drm_mode_to_intf_timing_params(). Otherwise, we get below error. > > I guess the interface timing configuration differ in silicon, thus the > software shall handle the > difference. If we adjust the width beforehand for DSI, we get below error. > > [ 6.625446] [drm:dpu_encoder_frame_done_timeout:2469] [dpu > error]enc31 frame done timeout > [ 6.642369] [drm:dpu_encoder_phys_vid_wait_for_commit_done:525] > [dpu error]vblank timeout: 4200c1 > [ 6.642395] [drm:dpu_kms_wait_for_commit_done:493] [dpu error]wait > for commit done returned -110 > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
On 5/27/2024 7:21 AM, Jun Nie wrote: > data is valid for only half the active window if widebus > is enabled > > Signed-off-by: Jun Nie <jun.nie@linaro.org> Hi Jun, Reviewed-by: Jessica Zhang <quic_jesszhan@quicinc.com> Thanks, Jessica Zhang > --- > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > 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 225c1c7768ff..f97221423249 100644 > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c > @@ -168,6 +168,15 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *intf, > > data_width = p->width; > > + /* > + * If widebus is enabled, data is valid for only half the active window > + * since the data rate is doubled in this mode. But for the compression > + * mode in DP case, the p->width is already adjusted in > + * drm_mode_to_intf_timing_params() > + */ > + if (p->wide_bus_en && !dp_intf) > + data_width = p->width >> 1; > + > hsync_data_start_x = hsync_start_x; > hsync_data_end_x = hsync_start_x + data_width - 1; > > > -- > 2.34.1 >
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 225c1c7768ff..f97221423249 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -168,6 +168,15 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *intf, data_width = p->width; + /* + * If widebus is enabled, data is valid for only half the active window + * since the data rate is doubled in this mode. But for the compression + * mode in DP case, the p->width is already adjusted in + * drm_mode_to_intf_timing_params() + */ + if (p->wide_bus_en && !dp_intf) + data_width = p->width >> 1; + hsync_data_start_x = hsync_start_x; hsync_data_end_x = hsync_start_x + data_width - 1;
data is valid for only half the active window if widebus is enabled Signed-off-by: Jun Nie <jun.nie@linaro.org> --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 9 +++++++++ 1 file changed, 9 insertions(+)