This is follow up update to Jonathan's patch set.
Changes vs V2:
- Rebase to latest mainline.
- 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 <[email protected]>
---
Jonathan Marek (5):
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 (fix video mode DSC)
drm/msm/dsi: add a comment to explain pkt_per_line encoding
drm/msm/dsi: support DSC configurations with slice_per_pkt > 1
Jun Nie (1):
drm/display: Add slice_per_pkt for dsc
.../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 +++++
drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 8 +++++
drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 +
drivers/gpu/drm/msm/dsi/dsi_host.c | 42 +++++++++++-----------
include/drm/display/drm_dsc.h | 4 +++
5 files changed, 44 insertions(+), 20 deletions(-)
---
base-commit: b1e6ec0a0fd0252af046e542f91234cd6c30b2cb
change-id: 20240403-msm-drm-dsc-dsi-video-upstream-1156d110a53d
Best regards,
--
Jun Nie <[email protected]>
Add variable for slice number of a DSC compression bit stream packet.
Its value shall be specified in panel driver, or default value can be set
in display controller driver if panel driver does not set it.
Signed-off-by: Jun Nie <[email protected]>
---
include/drm/display/drm_dsc.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/include/drm/display/drm_dsc.h b/include/drm/display/drm_dsc.h
index bc90273d06a6..4fac0a2746ae 100644
--- a/include/drm/display/drm_dsc.h
+++ b/include/drm/display/drm_dsc.h
@@ -82,6 +82,10 @@ struct drm_dsc_config {
* @bits_per_component: Bits per component to code (8/10/12)
*/
u8 bits_per_component;
+ /**
+ * @slice_per_pkt: slice number per DSC bit stream packet
+ */
+ u8 slice_per_pkt;
/**
* @convert_rgb:
* Flag to indicate if RGB - YCoCg conversion is needed
--
2.34.1
From: Jonathan Marek <[email protected]>
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 <[email protected]>
Reviewed-by: Dmitry Baryshkov <[email protected]>
---
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 2a0422cad6de..80ea4f1d8274 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -858,6 +858,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
@@ -865,6 +866,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;
@@ -902,6 +904,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
On Wed, 3 Apr 2024 at 12:11, Jun Nie <[email protected]> wrote:
>
> Add variable for slice number of a DSC compression bit stream packet.
> Its value shall be specified in panel driver, or default value can be set
> in display controller driver if panel driver does not set it.
This is not a part of the standard. Please justify it.
>
> Signed-off-by: Jun Nie <[email protected]>
> ---
> include/drm/display/drm_dsc.h | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/include/drm/display/drm_dsc.h b/include/drm/display/drm_dsc.h
> index bc90273d06a6..4fac0a2746ae 100644
> --- a/include/drm/display/drm_dsc.h
> +++ b/include/drm/display/drm_dsc.h
> @@ -82,6 +82,10 @@ struct drm_dsc_config {
> * @bits_per_component: Bits per component to code (8/10/12)
> */
> u8 bits_per_component;
> + /**
> + * @slice_per_pkt: slice number per DSC bit stream packet
> + */
> + u8 slice_per_pkt;
> /**
> * @convert_rgb:
> * Flag to indicate if RGB - YCoCg conversion is needed
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
On Wed, 3 Apr 2024 at 12:11, Jun Nie <[email protected]> wrote:
>
> This is follow up update to Jonathan's patch set.
>
> Changes vs V2:
> - Rebase to latest mainline.
> - 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.
Which comments? "Adress comments" is the worst case of changelog.
Also, what do you consider as version 2? Jonathan Marek has only sent v1.
>
> Signed-off-by: Jun Nie <[email protected]>
> ---
> Jonathan Marek (5):
> 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 (fix video mode DSC)
> drm/msm/dsi: add a comment to explain pkt_per_line encoding
> drm/msm/dsi: support DSC configurations with slice_per_pkt > 1
>
> Jun Nie (1):
> drm/display: Add slice_per_pkt for dsc
>
> .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 +++++
> drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 8 +++++
> drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 +
> drivers/gpu/drm/msm/dsi/dsi_host.c | 42 +++++++++++-----------
> include/drm/display/drm_dsc.h | 4 +++
> 5 files changed, 44 insertions(+), 20 deletions(-)
> ---
> base-commit: b1e6ec0a0fd0252af046e542f91234cd6c30b2cb
> change-id: 20240403-msm-drm-dsc-dsi-video-upstream-1156d110a53d
>
> Best regards,
> --
> Jun Nie <[email protected]>
>
--
With best wishes
Dmitry
On Wed, 3 Apr 2024 at 12:11, Jun Nie <[email protected]> wrote:
>
> From: Jonathan Marek <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
You S-o-b is missing
> ---
> 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 2a0422cad6de..80ea4f1d8274 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -858,6 +858,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
> @@ -865,6 +866,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;
>
> @@ -902,6 +904,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
>
--
With best wishes
Dmitry
Dmitry Baryshkov <[email protected]> 于2024年4月3日周三 17:49写道：
>
> On Wed, 3 Apr 2024 at 12:11, Jun Nie <[email protected]> wrote:
> >
> > This is follow up update to Jonathan's patch set.
> >
> > Changes vs V2:
> > - Rebase to latest mainline.
> > - 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.
>
> Which comments? "Adress comments" is the worst case of changelog.
Adopted. Will add more details in next version.
>
> Also, what do you consider as version 2? Jonathan Marek has only sent v1.
It's wired. I see v2 in patch title of below link. Just notice that
there is v1 in the link address.
https://patchwork.freedesktop.org/patch/567518/?series=126430&rev=1
>
> >
> > Signed-off-by: Jun Nie <[email protected]>
> > ---
> > Jonathan Marek (5):
> > 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 (fix video mode DSC)
> > drm/msm/dsi: add a comment to explain pkt_per_line encoding
> > drm/msm/dsi: support DSC configurations with slice_per_pkt > 1
> >
> > Jun Nie (1):
> > drm/display: Add slice_per_pkt for dsc
> >
> > .../gpu/drm/msm/disp/dpu1/dpu_encoder_phys_vid.c | 9 +++++
> > drivers/gpu/drm/msm/disp/dpu1/dpu_hw_intf.c | 8 +++++
> > drivers/gpu/drm/msm/dsi/dsi.xml.h | 1 +
> > drivers/gpu/drm/msm/dsi/dsi_host.c | 42 +++++++++++-----------
> > include/drm/display/drm_dsc.h | 4 +++
> > 5 files changed, 44 insertions(+), 20 deletions(-)
> > ---
> > base-commit: b1e6ec0a0fd0252af046e542f91234cd6c30b2cb
> > change-id: 20240403-msm-drm-dsc-dsi-video-upstream-1156d110a53d
> >
> > Best regards,
> > --
> > Jun Nie <[email protected]>
> >
>
>
> --
> With best wishes
> Dmitry
On Wed, 3 Apr 2024 at 17:27, Jun Nie <[email protected]> wrote:
>
> Dmitry Baryshkov <[email protected]> 于2024年4月3日周三 17:49写道：
> >
> > On Wed, 3 Apr 2024 at 12:11, Jun Nie <[email protected]> wrote:
> > >
> > > This is follow up update to Jonathan's patch set.
> > >
> > > Changes vs V2:
> > > - Rebase to latest mainline.
> > > - 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.
> >
> > Which comments? "Adress comments" is the worst case of changelog.
> Adopted. Will add more details in next version.
> >
> > Also, what do you consider as version 2? Jonathan Marek has only sent v1.
>
> It's wired. I see v2 in patch title of below link. Just notice that
> there is v1 in the link address.
> https://patchwork.freedesktop.org/patch/567518/?series=126430&rev=1
>
Ack, I didn't remember that there was v2. Please excuse me then.
--
With best wishes
Dmitry
Dmitry Baryshkov <[email protected]> 于2024年4月3日周三 17:41写道：
>
> On Wed, 3 Apr 2024 at 12:11, Jun Nie <[email protected]> wrote:
> >
> > Add variable for slice number of a DSC compression bit stream packet.
> > Its value shall be specified in panel driver, or default value can be set
> > in display controller driver if panel driver does not set it.
>
> This is not a part of the standard. Please justify it.
Right, I read the standard but did not find any details of packet description.
Looks like msm silicon support tuning of number of slice packing per downstream
code.
The slice_per_pkt can be set in the downstream msm device tree. And I test the
values 1 and 2 on vtdr6130 panel and both work. So I guess this is related to
performance or something like that. I will have more test with different panel
to check the impact.
drivers/gpu/drm/panel/panel-raydium-rm692e5.c also mentions to pass new value
to slice_per_pkt.
Hi Konrad,
Do you remember why value 2 is TODO for slice_per_pkt for panel rm692e5?
>
> >
> > Signed-off-by: Jun Nie <[email protected]>
> > ---
> > include/drm/display/drm_dsc.h | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/include/drm/display/drm_dsc.h b/include/drm/display/drm_dsc.h
> > index bc90273d06a6..4fac0a2746ae 100644
> > --- a/include/drm/display/drm_dsc.h
> > +++ b/include/drm/display/drm_dsc.h
> > @@ -82,6 +82,10 @@ struct drm_dsc_config {
> > * @bits_per_component: Bits per component to code (8/10/12)
> > */
> > u8 bits_per_component;
> > + /**
> > + * @slice_per_pkt: slice number per DSC bit stream packet
> > + */
> > + u8 slice_per_pkt;
> > /**
> > * @convert_rgb:
> > * Flag to indicate if RGB - YCoCg conversion is needed
> >
> > --
> > 2.34.1
> >
>
>
> --
> With best wishes
> Dmitry
On 2024-04-08 17:58:29, Jun Nie wrote:
> Dmitry Baryshkov <[email protected]> 于2024年4月3日周三 17:41写道：
> >
> > On Wed, 3 Apr 2024 at 12:11, Jun Nie <[email protected]> wrote:
> > >
> > > Add variable for slice number of a DSC compression bit stream packet.
> > > Its value shall be specified in panel driver, or default value can be set
> > > in display controller driver if panel driver does not set it.
> >
> > This is not a part of the standard. Please justify it.
>
> Right, I read the standard but did not find any details of packet description.
> Looks like msm silicon support tuning of number of slice packing per downstream
> code.
> The slice_per_pkt can be set in the downstream msm device tree. And I test the
> values 1 and 2 on vtdr6130 panel and both work. So I guess this is related to
> performance or something like that. I will have more test with different panel
> to check the impact.
> drivers/gpu/drm/panel/panel-raydium-rm692e5.c also mentions to pass new value
> to slice_per_pkt.
>
> Hi Konrad,
> Do you remember why value 2 is TODO for slice_per_pkt for panel rm692e5?
Hi Jun,
I think I should indirectly answer that question, as I indirectly via "the"
MDSS panel generator place that comment there based on the suggested downstream
value:
https://github.com/msm8916-mainline/linux-mdss-dsi-panel-driver-generator/commit/5c82e613d987d05feca423412f6de625f9c99bae#diff-dba3766d7cec900b8de500f888c64a392cd9780f9baf00aae7e3f87a7d3fefc4R458
So I don't think Konrad's answer will be any different than "that's what
downstream does, and that's what the generator put there".
---
I was fairly certain that it used for performance reasons, but panels were found
(e.g. on the FairPhone 5) that don't seem to function without combining multiple
(2) slices in one packet at all?
- Marijn
> > > Signed-off-by: Jun Nie <[email protected]>
> > > ---
> > > include/drm/display/drm_dsc.h | 4 ++++
> > > 1 file changed, 4 insertions(+)
> > >
> > > diff --git a/include/drm/display/drm_dsc.h b/include/drm/display/drm_dsc.h
> > > index bc90273d06a6..4fac0a2746ae 100644
> > > --- a/include/drm/display/drm_dsc.h
> > > +++ b/include/drm/display/drm_dsc.h
> > > @@ -82,6 +82,10 @@ struct drm_dsc_config {
> > > * @bits_per_component: Bits per component to code (8/10/12)
> > > */
> > > u8 bits_per_component;
> > > + /**
> > > + * @slice_per_pkt: slice number per DSC bit stream packet
> > > + */
> > > + u8 slice_per_pkt;
> > > /**
> > > * @convert_rgb:
> > > * Flag to indicate if RGB - YCoCg conversion is needed
> > >
> > > --
> > > 2.34.1
> > >
> >
> >
> > --
> > With best wishes
> > Dmitry
Can we drop (fix video mode DSC) from this patch title? It looks like more
patches are required to get this done, such a mention is more something for the
cover letter.
We could also clarify further to "set Word Count for video-mode DSC".
- Marijn
On 2024-04-03 17:10:59, Jun Nie wrote:
> From: Jonathan Marek <[email protected]>
>
> 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 <[email protected]>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> ---
> 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 2a0422cad6de..80ea4f1d8274 100644
> --- a/drivers/gpu/drm/msm/dsi/dsi_host.c
> +++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
> @@ -858,6 +858,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
> @@ -865,6 +866,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;
>
> @@ -902,6 +904,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
>
Marijn Suijten <[email protected]> 于2024年4月9日周二 00:45写道：
>
> Can we drop (fix video mode DSC) from this patch title? It looks like more
> patches are required to get this done, such a mention is more something for the
> cover letter.
>
> We could also clarify further to "set Word Count for video-mode DSC".
>
Accepted. video-mode DSC is achieved with the patch set, not this
specific patch.