Re: [BUG] Build failure and alleged fix for next-20240523
On 24/05/2024 20:57, Abhinav Kumar wrote:
Hello
On 5/24/2024 12:55 PM, Paul E. McKenney wrote:
Hello!
I get the following allmodconfig build error on x86 in next-20240523:
Traceback (most recent call last):
File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in
main()
File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main
parser.add_argument('--validate',
action=argparse.BooleanOptionalAction)
AttributeError: module 'argparse' has no attribute
'BooleanOptionalAction'
The following patch allows the build to complete successfully:
https://patchwork.kernel.org/project/dri-devel/patch/20240508091751.336654-1-jonath...@nvidia.com/#25842751
As to whether this is a proper fix, I must defer to the DRM folks on CC.
Thanx, Paul
Thanks for the report.
I have raised a merge request for
https://patchwork.freedesktop.org/patch/593057/ to make it available for
the next fixes release for msm.
This is also now in the mainline and so would be great to get this into
both -next and mainline.
Jon
--
nvpublic
Re: [PATCH] drm/msm/dpu: drop duplicate drm formats from wb2_formats arrays
On Fri, May 24, 2024 at 11:19:41AM -0700, Abhinav Kumar wrote:
>
>
> On 5/24/2024 8:01 AM, Junhao Xie wrote:
> > There are duplicate items in wb2_formats_rgb and wb2_formats_rgb_yuv,
> > which cause weston assertions failed.
> >
> > weston: libweston/drm-formats.c:131: weston_drm_format_array_add_format:
> > Assertion `!weston_drm_format_array_find_format(formats, format)' failed.
> >
> > Signed-off-by: Junhao Xie
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 6 --
> > 1 file changed, 6 deletions(-)
> >
>
> I think we need two fixes tag here, one for the RGB array and the other one
> for the RGB+YUV array.
>
> Fixes: 8c16b988ba2d ("drm/msm/dpu: introduce separate wb2_format arrays for
> rgb and yuv")
>
> Fixes: 53324b99bd7b ("drm/msm/dpu: add writeback blocks to the sm8250 DPU
> catalog")
>
> Reviewed-by: Abhinav Kumar
>
> (pls ignore the line breaks in the fixes line, I will fix it while applying)
With the Fixes tags in place:
Reviewed-by: Dmitry Baryshkov
--
With best wishes
Dmitry
Re: [PATCH v2] Revert "drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set"
On Fri, May 24, 2024 at 12:58:53PM -0700, Abhinav Kumar wrote:
>
>
> On 5/22/2024 3:24 AM, Dmitry Baryshkov wrote:
> > In the DPU driver blank IRQ handling is called from a vblank worker and
> > can happen outside of the irq_enable / irq_disable pair. Using the
> > worker makes that completely asynchronous with the rest of the code.
> > Revert commit d13f638c9b88 ("drm/msm/dpu: drop
> > dpu_encoder_phys_ops.atomic_mode_set") to fix vblank IRQ assignment for
> > CMD DSI panels.
> >
> > Call trace:
> > dpu_encoder_phys_cmd_control_vblank_irq+0x218/0x294
> >dpu_encoder_toggle_vblank_for_crtc+0x160/0x194
> >dpu_crtc_vblank+0xbc/0x228
> >dpu_kms_enable_vblank+0x18/0x24
> >vblank_ctrl_worker+0x34/0x6c
> >process_one_work+0x218/0x620
> >worker_thread+0x1ac/0x37c
> >kthread+0x114/0x118
> >ret_from_fork+0x10/0x20
> >
>
> Thanks for the stack.
>
> Agreed that vblank can be controlled asynchronously through the worker.
>
> And I am guessing that the worker thread ran and printed this error message
> because phys_enc->irq[INTR_IDX_VSYNC] was not valid as modeset had not
> happened by then?
modeset happened, but the vblanks can be enabled and disabled
afterwards.
>
> 272 end:
> 273 if (ret) {
> 274 DRM_ERROR("vblank irq err id:%u pp:%d ret:%d, enable %s/%d\n",
> 275 DRMID(phys_enc->parent),
> 276 phys_enc->hw_pp->idx - PINGPONG_0, ret,
> 277 enable ? "true" : "false", refcount);
> 278 }
>
> But how come this did not happen even with this revert.
>
> IOW, I am still missing how moving the irq assignment back to modeset from
> enable is fixing this?
It removes clearing of the IRQ fields in the irq_disable path, which
removes a requirement that vblank IRQ manipulations are called only
within the irq_enable/irq_disable brackets. I didn't have time to debug
this further on, so I think it's better to revert it now and return to
this cleanup later.
>
> > Fixes: d13f638c9b88 ("drm/msm/dpu: drop
> > dpu_encoder_phys_ops.atomic_mode_set")
> > Signed-off-by: Dmitry Baryshkov
> > ---
> > Changes in v2:
> > - Expanded commit message to describe the reason for revert and added a
> >call trace (Abhinav)
> > - Link to v1:
> > https://lore.kernel.org/r/20240514-dpu-revert-ams-v1-1-b13623d6c...@linaro.org
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 2 ++
> > drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 5
> > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 32
> > --
> > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 13 +++--
> > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 11 +++-
> > 5 files changed, 46 insertions(+), 17 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > index 119f3ea50a7c..a7d8ecf3f5be 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > @@ -1200,6 +1200,8 @@ static void dpu_encoder_virt_atomic_mode_set(struct
> > drm_encoder *drm_enc,
> > phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
> > phys->cached_mode = crtc_state->adjusted_mode;
> > + if (phys->ops.atomic_mode_set)
> > + phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
> > }
> > }
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> > index 002e89cc1705..30470cd15a48 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> > @@ -69,6 +69,8 @@ struct dpu_encoder_phys;
> >* @is_master:Whether this phys_enc is the current
> > master
> >*encoder. Can be switched at enable
> > time. Based
> >*on split_role and current mode
> > (CMD/VID).
> > + * @atomic_mode_set: DRM Call. Set a DRM mode.
> > + * This likely caches the mode, for use at enable.
> >* @enable: DRM Call. Enable a DRM mode.
> >* @disable: DRM Call. Disable mode.
> >* @control_vblank_irqRegister/Deregister for VBLANK IRQ
> > @@ -93,6 +95,9 @@ struct dpu_encoder_phys;
> > struct dpu_encoder_phys_ops {
> > void (*prepare_commit)(struct dpu_encoder_phys *encoder);
> > bool (*is_master)(struct dpu_encoder_phys *encoder);
> > + void (*atomic_mode_set)(struct dpu_encoder_phys *encoder,
> > + struct drm_crtc_state *crtc_state,
> > + struct drm_connector_state *conn_state);
> > void (*enable)(struct dpu_encoder_phys *encoder);
> > void (*disable)(struct dpu_encoder_phys *encoder);
> > int (*control_vblank_irq)(struct dpu_encoder_phys *enc, bool enable);
> > diff --git
Re: [PATCH v4 1/5] drm/msm/dpu: fix video mode DSC for DSI
On Fri, May 24, 2024 at 09:18:21PM +0800, Jun Nie wrote:
> From: Jonathan Marek
>
> Add necessary DPU timing and control changes for DSC to work with DSI
> video mode.
>
> Signed-off-by: Jonathan Marek
> Signed-off-by: Jun Nie
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 8
> drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 13 +
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 4
> 4 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> index 119f3ea50a7c..48cef6e79c70 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> @@ -564,7 +564,7 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder
> *drm_enc)
> return (num_dsc > 0) && (num_dsc > intf_count);
> }
>
> -static struct drm_dsc_config *dpu_encoder_get_dsc_config(struct drm_encoder
> *drm_enc)
> +struct drm_dsc_config *dpu_encoder_get_dsc_config(struct drm_encoder
> *drm_enc)
> {
> struct msm_drm_private *priv = drm_enc->dev->dev_private;
> struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> index 002e89cc1705..2167c46c1a45 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
> @@ -334,6 +334,14 @@ static inline enum dpu_3d_blend_mode
> dpu_encoder_helper_get_3d_blend_mode(
> */
> unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
>
> +/**
> + * dpu_encoder_get_dsc_config - get DSC config for the DPU encoder
> + * This helper function is used by physical encoder to get DSC config
> + * used for this encoder.
> + * @drm_enc: Pointer to encoder structure
> + */
> +struct drm_dsc_config *dpu_encoder_get_dsc_config(struct drm_encoder
> *drm_enc);
> +
> /**
> * dpu_encoder_get_drm_fmt - return DRM fourcc format
> * @phys_enc: Pointer to physical encoder structure
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> index ef69c2f408c3..7047b607ca91 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
> @@ -115,6 +115,19 @@ static void drm_mode_to_intf_timing_params(
> timing->h_front_porch = timing->h_front_porch >> 1;
> timing->hsync_pulse_width = timing->hsync_pulse_width >> 1;
> }
> +
> + /*
> + * for DSI, if compression is enabled, then divide the horizonal active
> + * timing parameters by compression ratio. bits of 3 components(R/G/B)
> + * is compressed into bits of 1 pixel.
> + */
> + if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
> + struct drm_dsc_config *dsc =
> +dpu_encoder_get_dsc_config(phys_enc->parent);
> + timing->width = timing->width * (dsc->bits_per_pixel >> 4) /
Here you are truncating fractional part of BPP. Please use
drm_dsc_get_bpp_int(), at least it will warn if there is a fractional
part. Or, even better, add a helper to calculate width * bpp / 64 / (bpc
* 3) and use it here and in dsi_adjust_pclk_for_compression()
> + (dsc->bits_per_component * 3);
> + timing->xres = timing->width;
> + }
> }
>
> static u32 get_horizontal_total(const struct dpu_hw_intf_timing_params
> *timing)
> 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..2cf1f8c116b5 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,10 @@ static void dpu_hw_intf_setup_timing_engine(struct
> dpu_hw_intf *intf,
>
> data_width = p->width;
>
> + /* TODO: handle DSC+DP case, we only handle DSC+DSI case so far */
> + if (p->compression_en && !dp_intf)
> + intf_cfg2 |= INTF_CFG2_DCE_DATA_COMPRESS;
> +
Separate commit, please
> hsync_data_start_x = hsync_start_x;
> hsync_data_end_x = hsync_start_x + data_width - 1;
>
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
Re: [PATCH v2] Revert "drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set"
On 5/22/2024 3:24 AM, Dmitry Baryshkov wrote:
In the DPU driver blank IRQ handling is called from a vblank worker and
can happen outside of the irq_enable / irq_disable pair. Using the
worker makes that completely asynchronous with the rest of the code.
Revert commit d13f638c9b88 ("drm/msm/dpu: drop
dpu_encoder_phys_ops.atomic_mode_set") to fix vblank IRQ assignment for
CMD DSI panels.
Call trace:
dpu_encoder_phys_cmd_control_vblank_irq+0x218/0x294
dpu_encoder_toggle_vblank_for_crtc+0x160/0x194
dpu_crtc_vblank+0xbc/0x228
dpu_kms_enable_vblank+0x18/0x24
vblank_ctrl_worker+0x34/0x6c
process_one_work+0x218/0x620
worker_thread+0x1ac/0x37c
kthread+0x114/0x118
ret_from_fork+0x10/0x20
Thanks for the stack.
Agreed that vblank can be controlled asynchronously through the worker.
And I am guessing that the worker thread ran and printed this error
message because phys_enc->irq[INTR_IDX_VSYNC] was not valid as modeset
had not happened by then?
272 end:
273 if (ret) {
274 DRM_ERROR("vblank irq err id:%u pp:%d ret:%d, enable %s/%d\n",
275 DRMID(phys_enc->parent),
276 phys_enc->hw_pp->idx - PINGPONG_0, ret,
277 enable ? "true" : "false", refcount);
278 }
But how come this did not happen even with this revert.
IOW, I am still missing how moving the irq assignment back to modeset
from enable is fixing this?
Fixes: d13f638c9b88 ("drm/msm/dpu: drop dpu_encoder_phys_ops.atomic_mode_set")
Signed-off-by: Dmitry Baryshkov
---
Changes in v2:
- Expanded commit message to describe the reason for revert and added a
call trace (Abhinav)
- Link to v1:
https://lore.kernel.org/r/20240514-dpu-revert-ams-v1-1-b13623d6c...@linaro.org
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c| 2 ++
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 5
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c | 32 --
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 13 +++--
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_wb.c| 11 +++-
5 files changed, 46 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..a7d8ecf3f5be 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -1200,6 +1200,8 @@ static void dpu_encoder_virt_atomic_mode_set(struct
drm_encoder *drm_enc,
phys->hw_ctl = to_dpu_hw_ctl(hw_ctl[i]);
phys->cached_mode = crtc_state->adjusted_mode;
+ if (phys->ops.atomic_mode_set)
+ phys->ops.atomic_mode_set(phys, crtc_state, conn_state);
}
}
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 002e89cc1705..30470cd15a48 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -69,6 +69,8 @@ struct dpu_encoder_phys;
* @is_master:Whether this phys_enc is the current
master
*encoder. Can be switched at enable time. Based
*on split_role and current mode (CMD/VID).
+ * @atomic_mode_set: DRM Call. Set a DRM mode.
+ * This likely caches the mode, for use at enable.
* @enable: DRM Call. Enable a DRM mode.
* @disable: DRM Call. Disable mode.
* @control_vblank_irqRegister/Deregister for VBLANK IRQ
@@ -93,6 +95,9 @@ struct dpu_encoder_phys;
struct dpu_encoder_phys_ops {
void (*prepare_commit)(struct dpu_encoder_phys *encoder);
bool (*is_master)(struct dpu_encoder_phys *encoder);
+ void (*atomic_mode_set)(struct dpu_encoder_phys *encoder,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state);
void (*enable)(struct dpu_encoder_phys *encoder);
void (*disable)(struct dpu_encoder_phys *encoder);
int (*control_vblank_irq)(struct dpu_encoder_phys *enc, bool enable);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
index 489be1c0c704..95cd39b49668 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_cmd.c
@@ -142,6 +142,23 @@ static void dpu_encoder_phys_cmd_underrun_irq(void *arg)
dpu_encoder_underrun_callback(phys_enc->parent, phys_enc);
}
+static void dpu_encoder_phys_cmd_atomic_mode_set(
+ struct dpu_encoder_phys *phys_enc,
+ struct drm_crtc_state *crtc_state,
+ struct drm_connector_state *conn_state)
+{
+ phys_enc->irq[INTR_IDX_CTL_START] = phys_enc->hw_ctl->caps->intr_start;
+
+ phys_enc->irq[INTR_IDX_PINGPONG] =
Re: [BUG] Build failure and alleged fix for next-20240523
Hello
On 5/24/2024 12:55 PM, Paul E. McKenney wrote:
Hello!
I get the following allmodconfig build error on x86 in next-20240523:
Traceback (most recent call last):
File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in
main()
File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main
parser.add_argument('--validate', action=argparse.BooleanOptionalAction)
AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction'
The following patch allows the build to complete successfully:
https://patchwork.kernel.org/project/dri-devel/patch/20240508091751.336654-1-jonath...@nvidia.com/#25842751
As to whether this is a proper fix, I must defer to the DRM folks on CC.
Thanx, Paul
Thanks for the report.
I have raised a merge request for
https://patchwork.freedesktop.org/patch/593057/ to make it available for
the next fixes release for msm.
Abhinav
[BUG] Build failure and alleged fix for next-20240523
Hello!
I get the following allmodconfig build error on x86 in next-20240523:
Traceback (most recent call last):
File "drivers/gpu/drm/msm/registers/gen_header.py", line 970, in
main()
File "drivers/gpu/drm/msm/registers/gen_header.py", line 951, in main
parser.add_argument('--validate', action=argparse.BooleanOptionalAction)
AttributeError: module 'argparse' has no attribute 'BooleanOptionalAction'
The following patch allows the build to complete successfully:
https://patchwork.kernel.org/project/dri-devel/patch/20240508091751.336654-1-jonath...@nvidia.com/#25842751
As to whether this is a proper fix, I must defer to the DRM folks on CC.
Thanx, Paul
Re: [PATCH] drm/msm/dpu: drop duplicate drm formats from wb2_formats arrays
On 5/24/2024 8:01 AM, Junhao Xie wrote:
There are duplicate items in wb2_formats_rgb and wb2_formats_rgb_yuv,
which cause weston assertions failed.
weston: libweston/drm-formats.c:131: weston_drm_format_array_add_format:
Assertion `!weston_drm_format_array_find_format(formats, format)' failed.
Signed-off-by: Junhao Xie
---
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 6 --
1 file changed, 6 deletions(-)
I think we need two fixes tag here, one for the RGB array and the other
one for the RGB+YUV array.
Fixes: 8c16b988ba2d ("drm/msm/dpu: introduce separate wb2_format arrays
for rgb and yuv")
Fixes: 53324b99bd7b ("drm/msm/dpu: add writeback blocks to the sm8250
DPU catalog")
Reviewed-by: Abhinav Kumar
(pls ignore the line breaks in the fixes line, I will fix it while applying)
Re: [PATCH] drm/msm/dpu: drop duplicate drm formats from wb2_formats arrays
On 24.05.2024 5:01 PM, Junhao Xie wrote: > There are duplicate items in wb2_formats_rgb and wb2_formats_rgb_yuv, > which cause weston assertions failed. > > weston: libweston/drm-formats.c:131: weston_drm_format_array_add_format: > Assertion `!weston_drm_format_array_find_format(formats, format)' failed. > > Signed-off-by: Junhao Xie > --- Reviewed-by: Konrad Dybcio Konrad
[PATCH v4 5/5] drm/msm/dsi: add a comment to explain pkt_per_line encoding
From: Jonathan Marek Make it clear why the pkt_per_line value is being "divided by 2". Signed-off-by: Jonathan Marek Reviewed-by: Dmitry Baryshkov Signed-off-by: Jun Nie --- drivers/gpu/drm/msm/dsi/dsi_host.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c index 7252d36687e6..4768cff08381 100644 --- a/drivers/gpu/drm/msm/dsi/dsi_host.c +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c @@ -885,7 +885,11 @@ static void dsi_update_dsc_timing(struct msm_dsi_host *msm_host, bool is_cmd_mod /* DSI_VIDEO_COMPRESSION_MODE & DSI_COMMAND_COMPRESSION_MODE * registers have similar offsets, so for below common code use * DSI_VIDEO_COMPRESSION_MODE_ for setting bits +* +* pkt_per_line is log2 encoded, >>1 works for supported values (1,2,4) */ + if (pkt_per_line > 4) + drm_warn_once(msm_host->dev, "pkt_per_line too big"); reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_PKT_PER_LINE(pkt_per_line >> 1); reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_EOL_BYTE_NUM(eol_byte_num); reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_EN; -- 2.34.1
[PATCH v4 4/5] drm/msm/dsi: set VIDEO_COMPRESSION_MODE_CTRL_WC
From: Jonathan Marek
Video mode DSC won't work if this field is not set correctly. Set it to fix
video mode DSC (for slice_per_pkt==1 cases at least).
Fixes: 08802f515c3c ("drm/msm/dsi: Add support for DSC configuration")
Signed-off-by: Jonathan Marek
Reviewed-by: Dmitry Baryshkov
Signed-off-by: Jun Nie
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 47f5858334f6..7252d36687e6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -857,6 +857,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
u32 slice_per_intf, total_bytes_per_intf;
u32 pkt_per_line;
u32 eol_byte_num;
+ u32 bytes_per_pkt;
/* first calculate dsc parameters and then program
* compress mode registers
@@ -864,6 +865,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
slice_per_intf = msm_dsc_get_slices_per_intf(dsc, hdisplay);
total_bytes_per_intf = dsc->slice_chunk_size * slice_per_intf;
+ bytes_per_pkt = dsc->slice_chunk_size; /* * slice_per_pkt; */
eol_byte_num = total_bytes_per_intf % 3;
@@ -901,6 +903,7 @@ static void dsi_update_dsc_timing(struct msm_dsi_host
*msm_host, bool is_cmd_mod
dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL,
reg_ctrl);
dsi_write(msm_host, REG_DSI_COMMAND_COMPRESSION_MODE_CTRL2,
reg_ctrl2);
} else {
+ reg |= DSI_VIDEO_COMPRESSION_MODE_CTRL_WC(bytes_per_pkt);
dsi_write(msm_host, REG_DSI_VIDEO_COMPRESSION_MODE_CTRL, reg);
}
}
--
2.34.1
[PATCH v4 3/5] drm/msm/dsi: set video mode widebus enable bit when widebus is enabled
From: Jonathan Marek
The value returned by msm_dsi_wide_bus_enabled() doesn't match what the
driver is doing in video mode. Fix that by actually enabling widebus for
video mode.
Fixes: efcbd6f9cdeb ("drm/msm/dsi: Enable widebus for DSI")
Signed-off-by: Jonathan Marek
Reviewed-by: Dmitry Baryshkov
Reviewed-by: Marijn Suijten
Reviewed-by: Jessica Zhang
Signed-off-by: Jun Nie
---
drivers/gpu/drm/msm/dsi/dsi_host.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c
b/drivers/gpu/drm/msm/dsi/dsi_host.c
index a50f4dda5941..47f5858334f6 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -754,6 +754,8 @@ static void dsi_ctrl_enable(struct msm_dsi_host *msm_host,
data |= DSI_VID_CFG0_TRAFFIC_MODE(dsi_get_traffic_mode(flags));
data |= DSI_VID_CFG0_DST_FORMAT(dsi_get_vid_fmt(mipi_fmt));
data |= DSI_VID_CFG0_VIRT_CHANNEL(msm_host->channel);
+ if (msm_dsi_host_is_wide_bus_enabled(_host->base))
+ data |= DSI_VID_CFG0_DATABUS_WIDEN;
dsi_write(msm_host, REG_DSI_VID_CFG0, data);
/* Do not swap RGB colors */
@@ -778,7 +780,6 @@ static void dsi_ctrl_enable(struct msm_dsi_host *msm_host,
if (cfg_hnd->minor >= MSM_DSI_6G_VER_MINOR_V1_3)
data |= DSI_CMD_MODE_MDP_CTRL2_BURST_MODE;
- /* TODO: Allow for video-mode support once tested/fixed
*/
if (msm_dsi_host_is_wide_bus_enabled(_host->base))
data |= DSI_CMD_MODE_MDP_CTRL2_DATABUS_WIDEN;
--
2.34.1
[PATCH v4 2/5] drm: adjust data width for widen bus case
data is valid for only half the active window if widebus is enabled Signed-off-by: Jun Nie --- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 8 1 file changed, 8 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 2cf1f8c116b5..3d1bc8fa4ca2 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c @@ -167,6 +167,14 @@ static void dpu_hw_intf_setup_timing_engine(struct dpu_hw_intf *intf, intf_cfg2 |= INTF_CFG2_DATABUS_WIDEN; 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; /* TODO: handle DSC+DP case, we only handle DSC+DSI case so far */ if (p->compression_en && !dp_intf) -- 2.34.1
[PATCH v4 1/5] drm/msm/dpu: fix video mode DSC for DSI
From: Jonathan Marek
Add necessary DPU timing and control changes for DSC to work with DSI
video mode.
Signed-off-by: Jonathan Marek
Signed-off-by: Jun Nie
---
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +-
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 8
drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 13 +
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 4
4 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 119f3ea50a7c..48cef6e79c70 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -564,7 +564,7 @@ bool dpu_encoder_use_dsc_merge(struct drm_encoder *drm_enc)
return (num_dsc > 0) && (num_dsc > intf_count);
}
-static struct drm_dsc_config *dpu_encoder_get_dsc_config(struct drm_encoder
*drm_enc)
+struct drm_dsc_config *dpu_encoder_get_dsc_config(struct drm_encoder *drm_enc)
{
struct msm_drm_private *priv = drm_enc->dev->dev_private;
struct dpu_encoder_virt *dpu_enc = to_dpu_encoder_virt(drm_enc);
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
index 002e89cc1705..2167c46c1a45 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h
@@ -334,6 +334,14 @@ static inline enum dpu_3d_blend_mode
dpu_encoder_helper_get_3d_blend_mode(
*/
unsigned int dpu_encoder_helper_get_dsc(struct dpu_encoder_phys *phys_enc);
+/**
+ * dpu_encoder_get_dsc_config - get DSC config for the DPU encoder
+ * This helper function is used by physical encoder to get DSC config
+ * used for this encoder.
+ * @drm_enc: Pointer to encoder structure
+ */
+struct drm_dsc_config *dpu_encoder_get_dsc_config(struct drm_encoder *drm_enc);
+
/**
* dpu_encoder_get_drm_fmt - return DRM fourcc format
* @phys_enc: Pointer to physical encoder structure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
index ef69c2f408c3..7047b607ca91 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c
@@ -115,6 +115,19 @@ static void drm_mode_to_intf_timing_params(
timing->h_front_porch = timing->h_front_porch >> 1;
timing->hsync_pulse_width = timing->hsync_pulse_width >> 1;
}
+
+ /*
+* for DSI, if compression is enabled, then divide the horizonal active
+* timing parameters by compression ratio. bits of 3 components(R/G/B)
+* is compressed into bits of 1 pixel.
+*/
+ if (phys_enc->hw_intf->cap->type != INTF_DP && timing->compression_en) {
+ struct drm_dsc_config *dsc =
+ dpu_encoder_get_dsc_config(phys_enc->parent);
+ timing->width = timing->width * (dsc->bits_per_pixel >> 4) /
+ (dsc->bits_per_component * 3);
+ timing->xres = timing->width;
+ }
}
static u32 get_horizontal_total(const struct dpu_hw_intf_timing_params *timing)
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..2cf1f8c116b5 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,10 @@ static void dpu_hw_intf_setup_timing_engine(struct
dpu_hw_intf *intf,
data_width = p->width;
+ /* TODO: handle DSC+DP case, we only handle DSC+DSI case so far */
+ if (p->compression_en && !dp_intf)
+ intf_cfg2 |= INTF_CFG2_DCE_DATA_COMPRESS;
+
hsync_data_start_x = hsync_start_x;
hsync_data_end_x = hsync_start_x + data_width - 1;
--
2.34.1
[PATCH v4 0/5] Add DSC support to DSI video panel
This is follow up update to Jonathan's patch set. Changes vs V3: - Rebase to latest msm-next-lumag branch. - Drop the slice_per_pkt change as it does impact basic DSC feature. - Remove change in generated dsi header - update DSC compressed width calculation with bpp and bpc - split wide bus impact on width into another patch - rename patch tile of VIDEO_COMPRESSION_MODE_CTRL_WC change - Polish warning usage - Add tags from reviewers Changes vs V2: - Drop the INTF_CFG2_DATA_HCTL_EN change as it is handled in latest mainline code. - Drop the bonded DSI patch as I do not have device to test it. - Address comments from version 2. Signed-off-by: Jun Nie --- Jonathan Marek (4): drm/msm/dpu: fix video mode DSC for DSI drm/msm/dsi: set video mode widebus enable bit when widebus is enabled drm/msm/dsi: set VIDEO_COMPRESSION_MODE_CTRL_WC drm/msm/dsi: add a comment to explain pkt_per_line encoding Jun Nie (1): drm: adjust data width for widen bus case drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 2 +- drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h | 8 drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 13 + drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 12 drivers/gpu/drm/msm/dsi/dsi_host.c | 10 +- 5 files changed, 43 insertions(+), 2 deletions(-) --- base-commit: e6428bcb611f6c164856a41fc5a1ae8471a9b5a9 change-id: 20240524-msm-drm-dsc-dsi-video-upstream-4-22e2266fbe89 Best regards, -- Jun Nie
