Re: [PATCH 11/17] drm/msm/dp: add VSC SDP support for YUV420 over DP
On 1/31/2024 8:36 PM, Dmitry Baryshkov wrote:
On Thu, 1 Feb 2024 at 03:56, Abhinav Kumar wrote:
On 1/27/2024 9:39 PM, Dmitry Baryshkov wrote:
On Sun, 28 Jan 2024 at 07:34, Paloma Arellano wrote:
On 1/25/2024 1:48 PM, Dmitry Baryshkov wrote:
On 25/01/2024 21:38, Paloma Arellano wrote:
Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 147
drivers/gpu/drm/msm/dp/dp_catalog.h | 4 +
drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
5 files changed, 205 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index c025786170ba5..7e4c68be23e56 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -29,6 +29,9 @@
#define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
+#define DP_GENERIC0_6_YUV_8_BPCBIT(0)
+#define DP_GENERIC0_6_YUV_10_BPCBIT(1)
+
#define DP_INTERRUPT_STATUS1 \
(DP_INTR_AUX_XFER_DONE| \
DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \
@@ -907,6 +910,150 @@ int dp_catalog_panel_timing_cfg(struct
dp_catalog *dp_catalog)
return 0;
}
+static void dp_catalog_panel_setup_vsc_sdp(struct dp_catalog
*dp_catalog)
+{
+struct dp_catalog_private *catalog;
+u32 header, parity, data;
+u8 bpc, off = 0;
+u8 buf[SZ_128];
+
+if (!dp_catalog) {
+pr_err("invalid input\n");
+return;
+}
+
+catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+/* HEADER BYTE 1 */
+header = dp_catalog->sdp.sdp_header.HB1;
+parity = dp_catalog_calculate_parity(header);
+data = ((header << HEADER_BYTE_1_BIT) | (parity <<
PARITY_BYTE_1_BIT));
+dp_write_link(catalog, MMSS_DP_GENERIC0_0, data);
+memcpy(buf + off, , sizeof(data));
+off += sizeof(data);
+
+/* HEADER BYTE 2 */
+header = dp_catalog->sdp.sdp_header.HB2;
+parity = dp_catalog_calculate_parity(header);
+data = ((header << HEADER_BYTE_2_BIT) | (parity <<
PARITY_BYTE_2_BIT));
+dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+
+/* HEADER BYTE 3 */
+header = dp_catalog->sdp.sdp_header.HB3;
+parity = dp_catalog_calculate_parity(header);
+data = ((header << HEADER_BYTE_3_BIT) | (parity <<
PARITY_BYTE_3_BIT));
+data |= dp_read_link(catalog, MMSS_DP_GENERIC0_1);
+dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+memcpy(buf + off, , sizeof(data));
+off += sizeof(data);
This seems to be common with the dp_audio code. Please extract this
header writing too.
These are two different sdp's. audio and vsc, are different with
different registers being written to and different amount of registers
being set. Can you please clarify since in audio we only need 3
registers to write to, and in vsc we need 10.
Bitmagic with the header is the same. Then the rest of the data is
written one dword per register, if I'm not mistaken.
We can generalize the MMSS_DP_GENERIC0 register writing by breaking it
up to two things:
1) Add a function vsc_sdp_pack() similar to hdmi_avi_infoframe_pack_only()
Note, there is already a hdmi_audio_infoframe_pack_for_dp() function.
I think this patchset can add hdmi_colorimetry_infoframe_pack_for_dp()
[you can choose any other similar name that suits from your POV].
Also please extract the function that inits the dp_sdp_header. It can
be reused as is for both existing hdmi_audio_infoframe_pack_for_dp(),
your new function and the dp_audio code.
Not sure if extracting the header will work as all other functions in
hdmi.c pack the header too so its a half and half implementation.
I am going to start with keeping this pack function in msm/dp/dp_utils.c
to start with.
If it gets to a form which is generic enough to keep in a helper which
can be common we can consider it once posted in v2.
2) dp_catalog_write_generic_pkt() which will just write the packed
buffer byte-by-byte to these MMSS_DP_GENERIC0_xxx register
But audio seems a bit different. We use DP_AUDIO_STREAM_0/1.
More importantly, it uses this sdp_map and writes each header one by one
with dp_catalog_audio_set_header().
Not sure if that entirely fits with this pack and then write model.
It can be simplified. But I dont think this effort is needed for this
series.
So I would prefer to generalize audio SDP programming separately.
I'd definitely ask to add a utility function that merges 4 header
bytes with the parity data. We already have 5 instances of that code
in dp_audio.c, which is already too much by the number of 4. Adding
the 6th copy is NAKed.
I acknowledge the overlap but coupling them with this feature doesn't
make sense as we have to individually test
Re: [PATCH 11/17] drm/msm/dp: add VSC SDP support for YUV420 over DP
On Thu, 1 Feb 2024 at 03:56, Abhinav Kumar wrote:
>
>
>
> On 1/27/2024 9:39 PM, Dmitry Baryshkov wrote:
> > On Sun, 28 Jan 2024 at 07:34, Paloma Arellano
> > wrote:
> >>
> >>
> >> On 1/25/2024 1:48 PM, Dmitry Baryshkov wrote:
> >>> On 25/01/2024 21:38, Paloma Arellano wrote:
> Add support to pack and send the VSC SDP packet for DP. This therefore
> allows the transmision of format information to the sinks which is
> needed for YUV420 support over DP.
>
> Signed-off-by: Paloma Arellano
> ---
> drivers/gpu/drm/msm/dp/dp_catalog.c | 147
> drivers/gpu/drm/msm/dp/dp_catalog.h | 4 +
> drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
> drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
> drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
> 5 files changed, 205 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index c025786170ba5..7e4c68be23e56 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -29,6 +29,9 @@
> #define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
> +#define DP_GENERIC0_6_YUV_8_BPCBIT(0)
> +#define DP_GENERIC0_6_YUV_10_BPCBIT(1)
> +
> #define DP_INTERRUPT_STATUS1 \
> (DP_INTR_AUX_XFER_DONE| \
> DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \
> @@ -907,6 +910,150 @@ int dp_catalog_panel_timing_cfg(struct
> dp_catalog *dp_catalog)
> return 0;
> }
> +static void dp_catalog_panel_setup_vsc_sdp(struct dp_catalog
> *dp_catalog)
> +{
> +struct dp_catalog_private *catalog;
> +u32 header, parity, data;
> +u8 bpc, off = 0;
> +u8 buf[SZ_128];
> +
> +if (!dp_catalog) {
> +pr_err("invalid input\n");
> +return;
> +}
> +
> +catalog = container_of(dp_catalog, struct dp_catalog_private,
> dp_catalog);
> +
> +/* HEADER BYTE 1 */
> +header = dp_catalog->sdp.sdp_header.HB1;
> +parity = dp_catalog_calculate_parity(header);
> +data = ((header << HEADER_BYTE_1_BIT) | (parity <<
> PARITY_BYTE_1_BIT));
> +dp_write_link(catalog, MMSS_DP_GENERIC0_0, data);
> +memcpy(buf + off, , sizeof(data));
> +off += sizeof(data);
> +
> +/* HEADER BYTE 2 */
> +header = dp_catalog->sdp.sdp_header.HB2;
> +parity = dp_catalog_calculate_parity(header);
> +data = ((header << HEADER_BYTE_2_BIT) | (parity <<
> PARITY_BYTE_2_BIT));
> +dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
> +
> +/* HEADER BYTE 3 */
> +header = dp_catalog->sdp.sdp_header.HB3;
> +parity = dp_catalog_calculate_parity(header);
> +data = ((header << HEADER_BYTE_3_BIT) | (parity <<
> PARITY_BYTE_3_BIT));
> +data |= dp_read_link(catalog, MMSS_DP_GENERIC0_1);
> +dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
> +memcpy(buf + off, , sizeof(data));
> +off += sizeof(data);
> >>>
> >>> This seems to be common with the dp_audio code. Please extract this
> >>> header writing too.
> >> These are two different sdp's. audio and vsc, are different with
> >> different registers being written to and different amount of registers
> >> being set. Can you please clarify since in audio we only need 3
> >> registers to write to, and in vsc we need 10.
> >
> > Bitmagic with the header is the same. Then the rest of the data is
> > written one dword per register, if I'm not mistaken.
> >
>
> We can generalize the MMSS_DP_GENERIC0 register writing by breaking it
> up to two things:
>
> 1) Add a function vsc_sdp_pack() similar to hdmi_avi_infoframe_pack_only()
Note, there is already a hdmi_audio_infoframe_pack_for_dp() function.
I think this patchset can add hdmi_colorimetry_infoframe_pack_for_dp()
[you can choose any other similar name that suits from your POV].
Also please extract the function that inits the dp_sdp_header. It can
be reused as is for both existing hdmi_audio_infoframe_pack_for_dp(),
your new function and the dp_audio code.
>
> 2) dp_catalog_write_generic_pkt() which will just write the packed
> buffer byte-by-byte to these MMSS_DP_GENERIC0_xxx register
>
> But audio seems a bit different. We use DP_AUDIO_STREAM_0/1.
> More importantly, it uses this sdp_map and writes each header one by one
> with dp_catalog_audio_set_header().
>
> Not sure if that entirely fits with this pack and then write model.
>
> It can be simplified. But I dont think this effort is needed for this
> series.
>
> So I would prefer to generalize audio SDP programming separately.
I'd definitely ask to add a utility function that merges 4 header
bytes with the parity data. We already have 5 instances of that code
in dp_audio.c, which is already
Re: [PATCH 11/17] drm/msm/dp: add VSC SDP support for YUV420 over DP
On 1/27/2024 9:39 PM, Dmitry Baryshkov wrote:
On Sun, 28 Jan 2024 at 07:34, Paloma Arellano wrote:
On 1/25/2024 1:48 PM, Dmitry Baryshkov wrote:
On 25/01/2024 21:38, Paloma Arellano wrote:
Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 147
drivers/gpu/drm/msm/dp/dp_catalog.h | 4 +
drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
5 files changed, 205 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index c025786170ba5..7e4c68be23e56 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -29,6 +29,9 @@
#define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
+#define DP_GENERIC0_6_YUV_8_BPCBIT(0)
+#define DP_GENERIC0_6_YUV_10_BPCBIT(1)
+
#define DP_INTERRUPT_STATUS1 \
(DP_INTR_AUX_XFER_DONE| \
DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \
@@ -907,6 +910,150 @@ int dp_catalog_panel_timing_cfg(struct
dp_catalog *dp_catalog)
return 0;
}
+static void dp_catalog_panel_setup_vsc_sdp(struct dp_catalog
*dp_catalog)
+{
+struct dp_catalog_private *catalog;
+u32 header, parity, data;
+u8 bpc, off = 0;
+u8 buf[SZ_128];
+
+if (!dp_catalog) {
+pr_err("invalid input\n");
+return;
+}
+
+catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+/* HEADER BYTE 1 */
+header = dp_catalog->sdp.sdp_header.HB1;
+parity = dp_catalog_calculate_parity(header);
+data = ((header << HEADER_BYTE_1_BIT) | (parity <<
PARITY_BYTE_1_BIT));
+dp_write_link(catalog, MMSS_DP_GENERIC0_0, data);
+memcpy(buf + off, , sizeof(data));
+off += sizeof(data);
+
+/* HEADER BYTE 2 */
+header = dp_catalog->sdp.sdp_header.HB2;
+parity = dp_catalog_calculate_parity(header);
+data = ((header << HEADER_BYTE_2_BIT) | (parity <<
PARITY_BYTE_2_BIT));
+dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+
+/* HEADER BYTE 3 */
+header = dp_catalog->sdp.sdp_header.HB3;
+parity = dp_catalog_calculate_parity(header);
+data = ((header << HEADER_BYTE_3_BIT) | (parity <<
PARITY_BYTE_3_BIT));
+data |= dp_read_link(catalog, MMSS_DP_GENERIC0_1);
+dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+memcpy(buf + off, , sizeof(data));
+off += sizeof(data);
This seems to be common with the dp_audio code. Please extract this
header writing too.
These are two different sdp's. audio and vsc, are different with
different registers being written to and different amount of registers
being set. Can you please clarify since in audio we only need 3
registers to write to, and in vsc we need 10.
Bitmagic with the header is the same. Then the rest of the data is
written one dword per register, if I'm not mistaken.
We can generalize the MMSS_DP_GENERIC0 register writing by breaking it
up to two things:
1) Add a function vsc_sdp_pack() similar to hdmi_avi_infoframe_pack_only()
2) dp_catalog_write_generic_pkt() which will just write the packed
buffer byte-by-byte to these MMSS_DP_GENERIC0_xxx register
But audio seems a bit different. We use DP_AUDIO_STREAM_0/1.
More importantly, it uses this sdp_map and writes each header one by one
with dp_catalog_audio_set_header().
Not sure if that entirely fits with this pack and then write model.
It can be simplified. But I dont think this effort is needed for this
series.
So I would prefer to generalize audio SDP programming separately.
+
+data = 0;
+dp_write_link(catalog, MMSS_DP_GENERIC0_2, data);
+memcpy(buf + off, , sizeof(data));
+off += sizeof(data);
Generally this is not how these functions are expected to be written.
Please take a look at drivers/video/hdmi.c. It should be split into:
- generic function that packs the C structure into a flat byte buffer,
- driver-specific function that formats and writes the buffer to the
hardware.
+dp_write_link(catalog, MMSS_DP_GENERIC0_3, data);
+memcpy(buf + off, , sizeof(data));
+off += sizeof(data);
+
+dp_write_link(catalog, MMSS_DP_GENERIC0_4, data);
+memcpy(buf + off, , sizeof(data));
+off += sizeof(data);
+
+dp_write_link(catalog, MMSS_DP_GENERIC0_5, data);
+memcpy(buf + off, , sizeof(data));
+off += sizeof(data);
+
+switch (dp_catalog->vsc_sdp_data.bpc) {
+case 10:
+bpc = DP_GENERIC0_6_YUV_10_BPC;
+break;
+case 8:
+default:
+bpc = DP_GENERIC0_6_YUV_8_BPC;
+break;
+}
+
+/* VSC SDP payload as per table 2-117 of DP 1.4 specification */
+data = (dp_catalog->vsc_sdp_data.colorimetry & 0xF) |
+ ((dp_catalog->vsc_sdp_data.pixelformat & 0xF) <<
Re: [PATCH 11/17] drm/msm/dp: add VSC SDP support for YUV420 over DP
On Sun, 28 Jan 2024 at 07:34, Paloma Arellano wrote:
>
>
> On 1/25/2024 1:48 PM, Dmitry Baryshkov wrote:
> > On 25/01/2024 21:38, Paloma Arellano wrote:
> >> Add support to pack and send the VSC SDP packet for DP. This therefore
> >> allows the transmision of format information to the sinks which is
> >> needed for YUV420 support over DP.
> >>
> >> Signed-off-by: Paloma Arellano
> >> ---
> >> drivers/gpu/drm/msm/dp/dp_catalog.c | 147
> >> drivers/gpu/drm/msm/dp/dp_catalog.h | 4 +
> >> drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
> >> drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
> >> drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
> >> 5 files changed, 205 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> index c025786170ba5..7e4c68be23e56 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> @@ -29,6 +29,9 @@
> >> #define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
> >> +#define DP_GENERIC0_6_YUV_8_BPCBIT(0)
> >> +#define DP_GENERIC0_6_YUV_10_BPCBIT(1)
> >> +
> >> #define DP_INTERRUPT_STATUS1 \
> >> (DP_INTR_AUX_XFER_DONE| \
> >> DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \
> >> @@ -907,6 +910,150 @@ int dp_catalog_panel_timing_cfg(struct
> >> dp_catalog *dp_catalog)
> >> return 0;
> >> }
> >> +static void dp_catalog_panel_setup_vsc_sdp(struct dp_catalog
> >> *dp_catalog)
> >> +{
> >> +struct dp_catalog_private *catalog;
> >> +u32 header, parity, data;
> >> +u8 bpc, off = 0;
> >> +u8 buf[SZ_128];
> >> +
> >> +if (!dp_catalog) {
> >> +pr_err("invalid input\n");
> >> +return;
> >> +}
> >> +
> >> +catalog = container_of(dp_catalog, struct dp_catalog_private,
> >> dp_catalog);
> >> +
> >> +/* HEADER BYTE 1 */
> >> +header = dp_catalog->sdp.sdp_header.HB1;
> >> +parity = dp_catalog_calculate_parity(header);
> >> +data = ((header << HEADER_BYTE_1_BIT) | (parity <<
> >> PARITY_BYTE_1_BIT));
> >> +dp_write_link(catalog, MMSS_DP_GENERIC0_0, data);
> >> +memcpy(buf + off, , sizeof(data));
> >> +off += sizeof(data);
> >> +
> >> +/* HEADER BYTE 2 */
> >> +header = dp_catalog->sdp.sdp_header.HB2;
> >> +parity = dp_catalog_calculate_parity(header);
> >> +data = ((header << HEADER_BYTE_2_BIT) | (parity <<
> >> PARITY_BYTE_2_BIT));
> >> +dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
> >> +
> >> +/* HEADER BYTE 3 */
> >> +header = dp_catalog->sdp.sdp_header.HB3;
> >> +parity = dp_catalog_calculate_parity(header);
> >> +data = ((header << HEADER_BYTE_3_BIT) | (parity <<
> >> PARITY_BYTE_3_BIT));
> >> +data |= dp_read_link(catalog, MMSS_DP_GENERIC0_1);
> >> +dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
> >> +memcpy(buf + off, , sizeof(data));
> >> +off += sizeof(data);
> >
> > This seems to be common with the dp_audio code. Please extract this
> > header writing too.
> These are two different sdp's. audio and vsc, are different with
> different registers being written to and different amount of registers
> being set. Can you please clarify since in audio we only need 3
> registers to write to, and in vsc we need 10.
Bitmagic with the header is the same. Then the rest of the data is
written one dword per register, if I'm not mistaken.
> >
> >> +
> >> +data = 0;
> >> +dp_write_link(catalog, MMSS_DP_GENERIC0_2, data);
> >> +memcpy(buf + off, , sizeof(data));
> >> +off += sizeof(data);
> >
> > Generally this is not how these functions are expected to be written.
> > Please take a look at drivers/video/hdmi.c. It should be split into:
> > - generic function that packs the C structure into a flat byte buffer,
> > - driver-specific function that formats and writes the buffer to the
> > hardware.
> >
> >> +dp_write_link(catalog, MMSS_DP_GENERIC0_3, data);
> >> +memcpy(buf + off, , sizeof(data));
> >> +off += sizeof(data);
> >> +
> >> +dp_write_link(catalog, MMSS_DP_GENERIC0_4, data);
> >> +memcpy(buf + off, , sizeof(data));
> >> +off += sizeof(data);
> >> +
> >> +dp_write_link(catalog, MMSS_DP_GENERIC0_5, data);
> >> +memcpy(buf + off, , sizeof(data));
> >> +off += sizeof(data);
> >> +
> >> +switch (dp_catalog->vsc_sdp_data.bpc) {
> >> +case 10:
> >> +bpc = DP_GENERIC0_6_YUV_10_BPC;
> >> +break;
> >> +case 8:
> >> +default:
> >> +bpc = DP_GENERIC0_6_YUV_8_BPC;
> >> +break;
> >> +}
> >> +
> >> +/* VSC SDP payload as per table 2-117 of DP 1.4 specification */
> >> +data = (dp_catalog->vsc_sdp_data.colorimetry & 0xF) |
> >> + ((dp_catalog->vsc_sdp_data.pixelformat & 0xF) << 4) |
> >> + (bpc << 8) |
> >> + ((dp_catalog->vsc_sdp_data.dynamic_range & 0x1) << 15) |
> >> + ((dp_catalog->vsc_sdp_data.content_type & 0x7) << 16);
> >> +
> >> +
Re: [PATCH 11/17] drm/msm/dp: add VSC SDP support for YUV420 over DP
On 1/25/2024 1:48 PM, Dmitry Baryshkov wrote:
On 25/01/2024 21:38, Paloma Arellano wrote:
Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 147
drivers/gpu/drm/msm/dp/dp_catalog.h | 4 +
drivers/gpu/drm/msm/dp/dp_ctrl.c | 4 +
drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
5 files changed, 205 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index c025786170ba5..7e4c68be23e56 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -29,6 +29,9 @@
#define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
+#define DP_GENERIC0_6_YUV_8_BPC BIT(0)
+#define DP_GENERIC0_6_YUV_10_BPC BIT(1)
+
#define DP_INTERRUPT_STATUS1 \
(DP_INTR_AUX_XFER_DONE| \
DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \
@@ -907,6 +910,150 @@ int dp_catalog_panel_timing_cfg(struct
dp_catalog *dp_catalog)
return 0;
}
+static void dp_catalog_panel_setup_vsc_sdp(struct dp_catalog
*dp_catalog)
+{
+ struct dp_catalog_private *catalog;
+ u32 header, parity, data;
+ u8 bpc, off = 0;
+ u8 buf[SZ_128];
+
+ if (!dp_catalog) {
+ pr_err("invalid input\n");
+ return;
+ }
+
+ catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+ /* HEADER BYTE 1 */
+ header = dp_catalog->sdp.sdp_header.HB1;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_1_BIT) | (parity <<
PARITY_BYTE_1_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_0, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ /* HEADER BYTE 2 */
+ header = dp_catalog->sdp.sdp_header.HB2;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_2_BIT) | (parity <<
PARITY_BYTE_2_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+
+ /* HEADER BYTE 3 */
+ header = dp_catalog->sdp.sdp_header.HB3;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_3_BIT) | (parity <<
PARITY_BYTE_3_BIT));
+ data |= dp_read_link(catalog, MMSS_DP_GENERIC0_1);
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
This seems to be common with the dp_audio code. Please extract this
header writing too.
These are two different sdp's. audio and vsc, are different with
different registers being written to and different amount of registers
being set. Can you please clarify since in audio we only need 3
registers to write to, and in vsc we need 10.
+
+ data = 0;
+ dp_write_link(catalog, MMSS_DP_GENERIC0_2, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
Generally this is not how these functions are expected to be written.
Please take a look at drivers/video/hdmi.c. It should be split into:
- generic function that packs the C structure into a flat byte buffer,
- driver-specific function that formats and writes the buffer to the
hardware.
+ dp_write_link(catalog, MMSS_DP_GENERIC0_3, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_4, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_5, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ switch (dp_catalog->vsc_sdp_data.bpc) {
+ case 10:
+ bpc = DP_GENERIC0_6_YUV_10_BPC;
+ break;
+ case 8:
+ default:
+ bpc = DP_GENERIC0_6_YUV_8_BPC;
+ break;
+ }
+
+ /* VSC SDP payload as per table 2-117 of DP 1.4 specification */
+ data = (dp_catalog->vsc_sdp_data.colorimetry & 0xF) |
+ ((dp_catalog->vsc_sdp_data.pixelformat & 0xF) << 4) |
+ (bpc << 8) |
+ ((dp_catalog->vsc_sdp_data.dynamic_range & 0x1) << 15) |
+ ((dp_catalog->vsc_sdp_data.content_type & 0x7) << 16);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_6, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ data = 0;
+ dp_write_link(catalog, MMSS_DP_GENERIC0_7, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_8, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_9, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ print_hex_dump(KERN_DEBUG, "[drm-dp] VSC: ", DUMP_PREFIX_NONE,
16, 4, buf, off, false);
+}
+
+void dp_catalog_panel_config_vsc_sdp(struct dp_catalog *dp_catalog,
bool en)
+{
+ struct dp_catalog_private *catalog;
+ u32
Re: [PATCH 11/17] drm/msm/dp: add VSC SDP support for YUV420 over DP
On 25/01/2024 21:38, Paloma Arellano wrote:
Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 147
drivers/gpu/drm/msm/dp/dp_catalog.h | 4 +
drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
5 files changed, 205 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index c025786170ba5..7e4c68be23e56 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -29,6 +29,9 @@
#define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
+#define DP_GENERIC0_6_YUV_8_BPC BIT(0)
+#define DP_GENERIC0_6_YUV_10_BPC BIT(1)
+
#define DP_INTERRUPT_STATUS1 \
(DP_INTR_AUX_XFER_DONE| \
DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \
@@ -907,6 +910,150 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
*dp_catalog)
return 0;
}
+static void dp_catalog_panel_setup_vsc_sdp(struct dp_catalog *dp_catalog)
+{
+ struct dp_catalog_private *catalog;
+ u32 header, parity, data;
+ u8 bpc, off = 0;
+ u8 buf[SZ_128];
+
+ if (!dp_catalog) {
+ pr_err("invalid input\n");
+ return;
+ }
+
+ catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+ /* HEADER BYTE 1 */
+ header = dp_catalog->sdp.sdp_header.HB1;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_1_BIT) | (parity <<
PARITY_BYTE_1_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_0, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ /* HEADER BYTE 2 */
+ header = dp_catalog->sdp.sdp_header.HB2;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_2_BIT) | (parity <<
PARITY_BYTE_2_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+
+ /* HEADER BYTE 3 */
+ header = dp_catalog->sdp.sdp_header.HB3;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_3_BIT) | (parity <<
PARITY_BYTE_3_BIT));
+ data |= dp_read_link(catalog, MMSS_DP_GENERIC0_1);
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
This seems to be common with the dp_audio code. Please extract this
header writing too.
+
+ data = 0;
+ dp_write_link(catalog, MMSS_DP_GENERIC0_2, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
Generally this is not how these functions are expected to be written.
Please take a look at drivers/video/hdmi.c. It should be split into:
- generic function that packs the C structure into a flat byte buffer,
- driver-specific function that formats and writes the buffer to the
hardware.
+ dp_write_link(catalog, MMSS_DP_GENERIC0_3, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_4, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_5, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ switch (dp_catalog->vsc_sdp_data.bpc) {
+ case 10:
+ bpc = DP_GENERIC0_6_YUV_10_BPC;
+ break;
+ case 8:
+ default:
+ bpc = DP_GENERIC0_6_YUV_8_BPC;
+ break;
+ }
+
+ /* VSC SDP payload as per table 2-117 of DP 1.4 specification */
+ data = (dp_catalog->vsc_sdp_data.colorimetry & 0xF) |
+ ((dp_catalog->vsc_sdp_data.pixelformat & 0xF) << 4) |
+ (bpc << 8) |
+ ((dp_catalog->vsc_sdp_data.dynamic_range & 0x1) << 15) |
+ ((dp_catalog->vsc_sdp_data.content_type & 0x7) << 16);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_6, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ data = 0;
+ dp_write_link(catalog, MMSS_DP_GENERIC0_7, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_8, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_9, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ print_hex_dump(KERN_DEBUG, "[drm-dp] VSC: ", DUMP_PREFIX_NONE, 16, 4,
buf, off, false);
+}
+
+void dp_catalog_panel_config_vsc_sdp(struct dp_catalog *dp_catalog, bool en)
+{
+ struct dp_catalog_private *catalog;
+ u32 cfg, cfg2, misc;
+ u16 major = 0, minor = 0;
+
+
[PATCH 11/17] drm/msm/dp: add VSC SDP support for YUV420 over DP
Add support to pack and send the VSC SDP packet for DP. This therefore
allows the transmision of format information to the sinks which is
needed for YUV420 support over DP.
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 147
drivers/gpu/drm/msm/dp/dp_catalog.h | 4 +
drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
drivers/gpu/drm/msm/dp/dp_panel.c | 47 +
drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
5 files changed, 205 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index c025786170ba5..7e4c68be23e56 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -29,6 +29,9 @@
#define DP_INTF_CONFIG_DATABUS_WIDEN BIT(4)
+#define DP_GENERIC0_6_YUV_8_BPCBIT(0)
+#define DP_GENERIC0_6_YUV_10_BPC BIT(1)
+
#define DP_INTERRUPT_STATUS1 \
(DP_INTR_AUX_XFER_DONE| \
DP_INTR_WRONG_ADDR | DP_INTR_TIMEOUT | \
@@ -907,6 +910,150 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
*dp_catalog)
return 0;
}
+static void dp_catalog_panel_setup_vsc_sdp(struct dp_catalog *dp_catalog)
+{
+ struct dp_catalog_private *catalog;
+ u32 header, parity, data;
+ u8 bpc, off = 0;
+ u8 buf[SZ_128];
+
+ if (!dp_catalog) {
+ pr_err("invalid input\n");
+ return;
+ }
+
+ catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+ /* HEADER BYTE 1 */
+ header = dp_catalog->sdp.sdp_header.HB1;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_1_BIT) | (parity <<
PARITY_BYTE_1_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_0, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ /* HEADER BYTE 2 */
+ header = dp_catalog->sdp.sdp_header.HB2;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_2_BIT) | (parity <<
PARITY_BYTE_2_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+
+ /* HEADER BYTE 3 */
+ header = dp_catalog->sdp.sdp_header.HB3;
+ parity = dp_catalog_calculate_parity(header);
+ data = ((header << HEADER_BYTE_3_BIT) | (parity <<
PARITY_BYTE_3_BIT));
+ data |= dp_read_link(catalog, MMSS_DP_GENERIC0_1);
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ data = 0;
+ dp_write_link(catalog, MMSS_DP_GENERIC0_2, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_3, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_4, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_5, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ switch (dp_catalog->vsc_sdp_data.bpc) {
+ case 10:
+ bpc = DP_GENERIC0_6_YUV_10_BPC;
+ break;
+ case 8:
+ default:
+ bpc = DP_GENERIC0_6_YUV_8_BPC;
+ break;
+ }
+
+ /* VSC SDP payload as per table 2-117 of DP 1.4 specification */
+ data = (dp_catalog->vsc_sdp_data.colorimetry & 0xF) |
+ ((dp_catalog->vsc_sdp_data.pixelformat & 0xF) << 4) |
+ (bpc << 8) |
+ ((dp_catalog->vsc_sdp_data.dynamic_range & 0x1) << 15) |
+ ((dp_catalog->vsc_sdp_data.content_type & 0x7) << 16);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_6, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ data = 0;
+ dp_write_link(catalog, MMSS_DP_GENERIC0_7, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_8, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ dp_write_link(catalog, MMSS_DP_GENERIC0_9, data);
+ memcpy(buf + off, , sizeof(data));
+ off += sizeof(data);
+
+ print_hex_dump(KERN_DEBUG, "[drm-dp] VSC: ", DUMP_PREFIX_NONE, 16, 4,
buf, off, false);
+}
+
+void dp_catalog_panel_config_vsc_sdp(struct dp_catalog *dp_catalog, bool en)
+{
+ struct dp_catalog_private *catalog;
+ u32 cfg, cfg2, misc;
+ u16 major = 0, minor = 0;
+
+ if (!dp_catalog) {
+ pr_err("invalid input\n");
+ return;
+ }
+
+ catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+ cfg = dp_read_link(catalog, MMSS_DP_SDP_CFG);
+ cfg2 = dp_read_link(catalog, MMSS_DP_SDP_CFG2);
+ misc = dp_read_link(catalog, REG_DP_MISC1_MISC0);
+
+ if (en) {
+ cfg |= GEN0_SDP_EN;
+
