[PATCH v3 13/19] 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.
Changes in v3:
- Create a new struct, msm_dp_sdp_with_parity, which holds the
packing information for VSC SDP
- Use drm_dp_vsc_sdp_pack() to pack the data into the new
msm_dp_sdp_with_parity struct instead of specifically packing
for YUV420 format
- Modify dp_catalog_panel_send_vsc_sdp() to send the VSC SDP
data using the new msm_dp_sdp_with_parity struct
Changes in v2:
- Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
- Remove dp_sdp from the dp_catalog struct since this data is
being allocated at the point used
- Create a new function in dp_utils to pack the VSC SDP data
into a buffer
- Create a new function that packs the SDP header bytes into a
buffer. This function is made generic so that it can be
utilized by dp_audio
header bytes into a buffer
- Create a new function in dp_utils that takes the packed buffer
and writes to the DP_GENERIC0_* registers
- Split the dp_catalog_panel_config_vsc_sdp() function into two
to disable/enable sending VSC SDP packets
- Check the DP HW version using the original useage of
dp_catalog_hw_revision() and correct the version checking
logic
- Rename dp_panel_setup_vsc_sdp() to
dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
currently VSC SDP is only being set up to support YUV420 modes
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 113
drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++
drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
drivers/gpu/drm/msm/dp/dp_panel.c | 55 ++
drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
drivers/gpu/drm/msm/dp/dp_utils.c | 48
drivers/gpu/drm/msm/dp/dp_utils.h | 18 +
7 files changed, 248 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 5d84c089e520a..61d5317efe683 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -901,6 +901,119 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
*dp_catalog)
return 0;
}
+static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog,
+ struct msm_dp_sdp_with_parity
*msm_dp_sdp)
+{
+ struct dp_catalog_private *catalog;
+ u32 val;
+
+ if (!dp_catalog) {
+ DRM_ERROR("invalid input\n");
+ return;
+ }
+
+ catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+ val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB0) << HEADER_BYTE_0_BIT |
+ (msm_dp_sdp->pb.PB0 << PARITY_BYTE_0_BIT) |
+ (msm_dp_sdp->vsc_sdp.sdp_header.HB1) << HEADER_BYTE_1_BIT |
+ (msm_dp_sdp->pb.PB1 << PARITY_BYTE_1_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_0, val);
+
+ val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB2) << HEADER_BYTE_2_BIT |
+ (msm_dp_sdp->pb.PB2 << PARITY_BYTE_2_BIT) |
+ (msm_dp_sdp->vsc_sdp.sdp_header.HB3) << HEADER_BYTE_3_BIT |
+ (msm_dp_sdp->pb.PB3 << PARITY_BYTE_3_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, val);
+
+ val = ((msm_dp_sdp->vsc_sdp.db[16]) | (msm_dp_sdp->vsc_sdp.db[17] << 8)
|
+ (msm_dp_sdp->vsc_sdp.db[18] << 16));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_6, val);
+}
+
+static void dp_catalog_panel_update_sdp(struct dp_catalog *dp_catalog)
+{
+ struct dp_catalog_private *catalog;
+ u32 hw_revision;
+
+ catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+ hw_revision = dp_catalog_hw_revision(dp_catalog);
+ if (hw_revision < DP_HW_VERSION_1_2 && hw_revision >=
DP_HW_VERSION_1_0) {
+ dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x01);
+ dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x00);
+ }
+}
+
+void dp_catalog_panel_enable_vsc_sdp(struct dp_catalog *dp_catalog,
+struct msm_dp_sdp_with_parity *msm_dp_sdp)
+{
+ struct dp_catalog_private *catalog;
+ u32 cfg, cfg2, misc;
+
+ if (!dp_catalog) {
+ DRM_ERROR("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);
+
+ cfg |= GEN0_SDP_EN;
+ dp_write_link(catalog, MMSS_DP_SDP_CFG, cfg);
+
+ cfg2 |= GENERIC0_SDPSIZE_VALID;
+ dp_write_link(catalog, MMSS_DP_SDP_CFG2, cfg2)
Re: [PATCH v3 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP
On Wed, 14 Feb 2024 at 20:04, 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.
>
> Changes in v3:
> - Create a new struct, msm_dp_sdp_with_parity, which holds the
> packing information for VSC SDP
> - Use drm_dp_vsc_sdp_pack() to pack the data into the new
> msm_dp_sdp_with_parity struct instead of specifically packing
> for YUV420 format
> - Modify dp_catalog_panel_send_vsc_sdp() to send the VSC SDP
> data using the new msm_dp_sdp_with_parity struct
>
> Changes in v2:
> - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
> - Remove dp_sdp from the dp_catalog struct since this data is
> being allocated at the point used
> - Create a new function in dp_utils to pack the VSC SDP data
> into a buffer
> - Create a new function that packs the SDP header bytes into a
> buffer. This function is made generic so that it can be
> utilized by dp_audio
> header bytes into a buffer
> - Create a new function in dp_utils that takes the packed buffer
> and writes to the DP_GENERIC0_* registers
> - Split the dp_catalog_panel_config_vsc_sdp() function into two
> to disable/enable sending VSC SDP packets
> - Check the DP HW version using the original useage of
> dp_catalog_hw_revision() and correct the version checking
> logic
> - Rename dp_panel_setup_vsc_sdp() to
> dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
> currently VSC SDP is only being set up to support YUV420 modes
>
> Signed-off-by: Paloma Arellano
> ---
> drivers/gpu/drm/msm/dp/dp_catalog.c | 113
> drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++
> drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
> drivers/gpu/drm/msm/dp/dp_panel.c | 55 ++
> drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
> drivers/gpu/drm/msm/dp/dp_utils.c | 48
> drivers/gpu/drm/msm/dp/dp_utils.h | 18 +
> 7 files changed, 248 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index 5d84c089e520a..61d5317efe683 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -901,6 +901,119 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
> *dp_catalog)
> return 0;
> }
>
> +static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog,
> + struct msm_dp_sdp_with_parity
> *msm_dp_sdp)
> +{
> + struct dp_catalog_private *catalog;
> + u32 val;
> +
> + if (!dp_catalog) {
> + DRM_ERROR("invalid input\n");
> + return;
> + }
> +
> + catalog = container_of(dp_catalog, struct dp_catalog_private,
> dp_catalog);
> +
> + val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB0) << HEADER_BYTE_0_BIT |
> + (msm_dp_sdp->pb.PB0 << PARITY_BYTE_0_BIT) |
> + (msm_dp_sdp->vsc_sdp.sdp_header.HB1) << HEADER_BYTE_1_BIT |
> + (msm_dp_sdp->pb.PB1 << PARITY_BYTE_1_BIT));
> + dp_write_link(catalog, MMSS_DP_GENERIC0_0, val);
> +
> + val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB2) << HEADER_BYTE_2_BIT |
> + (msm_dp_sdp->pb.PB2 << PARITY_BYTE_2_BIT) |
> + (msm_dp_sdp->vsc_sdp.sdp_header.HB3) << HEADER_BYTE_3_BIT |
> + (msm_dp_sdp->pb.PB3 << PARITY_BYTE_3_BIT));
> + dp_write_link(catalog, MMSS_DP_GENERIC0_1, val);
I still think that this is not the way to do it. Could you please
extract the function that takes struct dp_sdp_header, calculates
padding and writes resulting data to the hardware? This way we can
reuse it later for all the dp_audio stuff.
> +
> + val = ((msm_dp_sdp->vsc_sdp.db[16]) | (msm_dp_sdp->vsc_sdp.db[17] <<
> 8) |
> + (msm_dp_sdp->vsc_sdp.db[18] << 16));
> + dp_write_link(catalog, MMSS_DP_GENERIC0_6, val);
Shouldn't we write full dp_sdp data, including all zeroes? Here you
assume that there is no other data in dp_sdp and also that nobody
wrote anything senseless to those registers.
> +}
> +
> +static void dp_catalog_panel_update_sdp(struct dp_catalog *dp_catalog)
> +{
> + struct dp_catalog_private *catalog;
> + u32 hw_revision;
> +
> + catalog = container_of(dp_catalog, struct dp_catalog_private,
> dp_catalog);
> +
> + hw_revision = dp_catalog_hw_revision(dp_catalog);
> + if (hw_revision < DP_HW_VERSION_1_2 && hw_revision >=
> DP_HW_VERSION_1_0) {
> + dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x01);
> + dp_write_link(catalog, MMSS_DP_SDP_CFG3, 0x00);
> + }
> +}
> +
> +void dp_catalog_panel_enable_vsc_sdp(struct dp_catalog *dp_catalog,
> +
Re: [PATCH v3 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP
On 2/14/2024 11:39 AM, Dmitry Baryshkov wrote:
On Wed, 14 Feb 2024 at 20:04, 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.
Changes in v3:
- Create a new struct, msm_dp_sdp_with_parity, which holds the
packing information for VSC SDP
- Use drm_dp_vsc_sdp_pack() to pack the data into the new
msm_dp_sdp_with_parity struct instead of specifically packing
for YUV420 format
- Modify dp_catalog_panel_send_vsc_sdp() to send the VSC SDP
data using the new msm_dp_sdp_with_parity struct
Changes in v2:
- Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
- Remove dp_sdp from the dp_catalog struct since this data is
being allocated at the point used
- Create a new function in dp_utils to pack the VSC SDP data
into a buffer
- Create a new function that packs the SDP header bytes into a
buffer. This function is made generic so that it can be
utilized by dp_audio
header bytes into a buffer
- Create a new function in dp_utils that takes the packed buffer
and writes to the DP_GENERIC0_* registers
- Split the dp_catalog_panel_config_vsc_sdp() function into two
to disable/enable sending VSC SDP packets
- Check the DP HW version using the original useage of
dp_catalog_hw_revision() and correct the version checking
logic
- Rename dp_panel_setup_vsc_sdp() to
dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
currently VSC SDP is only being set up to support YUV420 modes
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 113
drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++
drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
drivers/gpu/drm/msm/dp/dp_panel.c | 55 ++
drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
drivers/gpu/drm/msm/dp/dp_utils.c | 48
drivers/gpu/drm/msm/dp/dp_utils.h | 18 +
7 files changed, 248 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 5d84c089e520a..61d5317efe683 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -901,6 +901,119 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
*dp_catalog)
return 0;
}
+static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog,
+ struct msm_dp_sdp_with_parity
*msm_dp_sdp)
+{
+ struct dp_catalog_private *catalog;
+ u32 val;
+
+ if (!dp_catalog) {
+ DRM_ERROR("invalid input\n");
+ return;
+ }
+
+ catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+ val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB0) << HEADER_BYTE_0_BIT |
+ (msm_dp_sdp->pb.PB0 << PARITY_BYTE_0_BIT) |
+ (msm_dp_sdp->vsc_sdp.sdp_header.HB1) << HEADER_BYTE_1_BIT |
+ (msm_dp_sdp->pb.PB1 << PARITY_BYTE_1_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_0, val);
+
+ val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB2) << HEADER_BYTE_2_BIT |
+ (msm_dp_sdp->pb.PB2 << PARITY_BYTE_2_BIT) |
+ (msm_dp_sdp->vsc_sdp.sdp_header.HB3) << HEADER_BYTE_3_BIT |
+ (msm_dp_sdp->pb.PB3 << PARITY_BYTE_3_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, val);
I still think that this is not the way to do it. Could you please
extract the function that takes struct dp_sdp_header, calculates
padding and writes resulting data to the hardware? This way we can
reuse it later for all the dp_audio stuff.
hmmm ... dp_utils_pack_sdp_header() does that you are asking for right?
OR are you asking for another function like:
1) rename dp_utils_pack_sdp_header() to dp_utils_calc_sdp_parity()
2) dp_utils_pack_sdp() takes two u32 to pack the header and parity
together and we move the << HEADER_BYTE_xx | part to it
dp_catalog_panel_send_vsc_sdp() just uses these two u32 to write the
headers.
+
+ val = ((msm_dp_sdp->vsc_sdp.db[16]) | (msm_dp_sdp->vsc_sdp.db[17] << 8)
|
+ (msm_dp_sdp->vsc_sdp.db[18] << 16));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_6, val);
Shouldn't we write full dp_sdp data, including all zeroes? Here you
assume that there is no other data in dp_sdp and also that nobody
wrote anything senseless to those registers.
As per documentation, it says db[0] to db[15] are reserved so I thought
its better not to touch/use them and start writing for 16 onwards.
1592 * VSC SDP Payload for Pixel Encoding/Colorimetry Format
1593 * db[0] - db[15]: Reserved
1594 * db[16]: Pixel Encoding and Colorimetry Formats
1595 * db[17]: Dynamic Range an
Re: [PATCH v3 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP
On Wed, 14 Feb 2024 at 22:15, Abhinav Kumar wrote:
>
>
>
> On 2/14/2024 11:39 AM, Dmitry Baryshkov wrote:
> > On Wed, 14 Feb 2024 at 20:04, 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.
> >>
> >> Changes in v3:
> >> - Create a new struct, msm_dp_sdp_with_parity, which holds the
> >>packing information for VSC SDP
> >> - Use drm_dp_vsc_sdp_pack() to pack the data into the new
> >>msm_dp_sdp_with_parity struct instead of specifically packing
> >>for YUV420 format
> >> - Modify dp_catalog_panel_send_vsc_sdp() to send the VSC SDP
> >>data using the new msm_dp_sdp_with_parity struct
> >>
> >> Changes in v2:
> >> - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
> >> - Remove dp_sdp from the dp_catalog struct since this data is
> >>being allocated at the point used
> >> - Create a new function in dp_utils to pack the VSC SDP data
> >>into a buffer
> >> - Create a new function that packs the SDP header bytes into a
> >>buffer. This function is made generic so that it can be
> >>utilized by dp_audio
> >>header bytes into a buffer
> >> - Create a new function in dp_utils that takes the packed buffer
> >>and writes to the DP_GENERIC0_* registers
> >> - Split the dp_catalog_panel_config_vsc_sdp() function into two
> >>to disable/enable sending VSC SDP packets
> >> - Check the DP HW version using the original useage of
> >>dp_catalog_hw_revision() and correct the version checking
> >>logic
> >> - Rename dp_panel_setup_vsc_sdp() to
> >>dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
> >>currently VSC SDP is only being set up to support YUV420 modes
> >>
> >> Signed-off-by: Paloma Arellano
> >> ---
> >> drivers/gpu/drm/msm/dp/dp_catalog.c | 113
> >> drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++
> >> drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
> >> drivers/gpu/drm/msm/dp/dp_panel.c | 55 ++
> >> drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
> >> drivers/gpu/drm/msm/dp/dp_utils.c | 48
> >> drivers/gpu/drm/msm/dp/dp_utils.h | 18 +
> >> 7 files changed, 248 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> index 5d84c089e520a..61d5317efe683 100644
> >> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> >> @@ -901,6 +901,119 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
> >> *dp_catalog)
> >> return 0;
> >> }
> >>
> >> +static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog,
> >> + struct msm_dp_sdp_with_parity
> >> *msm_dp_sdp)
> >> +{
> >> + struct dp_catalog_private *catalog;
> >> + u32 val;
> >> +
> >> + if (!dp_catalog) {
> >> + DRM_ERROR("invalid input\n");
> >> + return;
> >> + }
> >> +
> >> + catalog = container_of(dp_catalog, struct dp_catalog_private,
> >> dp_catalog);
> >> +
> >> + val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB0) << HEADER_BYTE_0_BIT |
> >> + (msm_dp_sdp->pb.PB0 << PARITY_BYTE_0_BIT) |
> >> + (msm_dp_sdp->vsc_sdp.sdp_header.HB1) << HEADER_BYTE_1_BIT |
> >> + (msm_dp_sdp->pb.PB1 << PARITY_BYTE_1_BIT));
> >> + dp_write_link(catalog, MMSS_DP_GENERIC0_0, val);
> >> +
> >> + val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB2) << HEADER_BYTE_2_BIT |
> >> + (msm_dp_sdp->pb.PB2 << PARITY_BYTE_2_BIT) |
> >> + (msm_dp_sdp->vsc_sdp.sdp_header.HB3) << HEADER_BYTE_3_BIT |
> >> + (msm_dp_sdp->pb.PB3 << PARITY_BYTE_3_BIT));
> >> + dp_write_link(catalog, MMSS_DP_GENERIC0_1, val);
> >
> > I still think that this is not the way to do it. Could you please
> > extract the function that takes struct dp_sdp_header, calculates
> > padding and writes resulting data to the hardware? This way we can
> > reuse it later for all the dp_audio stuff.
> >
>
> hmmm ... dp_utils_pack_sdp_header() does that you are asking for right?
>
> OR are you asking for another function like:
>
> 1) rename dp_utils_pack_sdp_header() to dp_utils_calc_sdp_parity()
> 2) dp_utils_pack_sdp() takes two u32 to pack the header and parity
> together and we move the << HEADER_BYTE_xx | part to it
>
> dp_catalog_panel_send_vsc_sdp() just uses these two u32 to write the
> headers.
I'm really looking for the following function:
void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog,
struct dp_sdp *dp_sdp)
{
dp_write_vsc_header(dp_catalog, MMSS_DP_GENERIC0_0, &dp_sdp-
Re: [PATCH v3 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP
On 2/15/2024 12:40 AM, Dmitry Baryshkov wrote:
On Wed, 14 Feb 2024 at 22:15, Abhinav Kumar wrote:
On 2/14/2024 11:39 AM, Dmitry Baryshkov wrote:
On Wed, 14 Feb 2024 at 20:04, 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.
Changes in v3:
- Create a new struct, msm_dp_sdp_with_parity, which holds the
packing information for VSC SDP
- Use drm_dp_vsc_sdp_pack() to pack the data into the new
msm_dp_sdp_with_parity struct instead of specifically packing
for YUV420 format
- Modify dp_catalog_panel_send_vsc_sdp() to send the VSC SDP
data using the new msm_dp_sdp_with_parity struct
Changes in v2:
- Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
- Remove dp_sdp from the dp_catalog struct since this data is
being allocated at the point used
- Create a new function in dp_utils to pack the VSC SDP data
into a buffer
- Create a new function that packs the SDP header bytes into a
buffer. This function is made generic so that it can be
utilized by dp_audio
header bytes into a buffer
- Create a new function in dp_utils that takes the packed buffer
and writes to the DP_GENERIC0_* registers
- Split the dp_catalog_panel_config_vsc_sdp() function into two
to disable/enable sending VSC SDP packets
- Check the DP HW version using the original useage of
dp_catalog_hw_revision() and correct the version checking
logic
- Rename dp_panel_setup_vsc_sdp() to
dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
currently VSC SDP is only being set up to support YUV420 modes
Signed-off-by: Paloma Arellano
---
drivers/gpu/drm/msm/dp/dp_catalog.c | 113
drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++
drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
drivers/gpu/drm/msm/dp/dp_panel.c | 55 ++
drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
drivers/gpu/drm/msm/dp/dp_utils.c | 48
drivers/gpu/drm/msm/dp/dp_utils.h | 18 +
7 files changed, 248 insertions(+)
diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
b/drivers/gpu/drm/msm/dp/dp_catalog.c
index 5d84c089e520a..61d5317efe683 100644
--- a/drivers/gpu/drm/msm/dp/dp_catalog.c
+++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
@@ -901,6 +901,119 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
*dp_catalog)
return 0;
}
+static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog,
+ struct msm_dp_sdp_with_parity
*msm_dp_sdp)
+{
+ struct dp_catalog_private *catalog;
+ u32 val;
+
+ if (!dp_catalog) {
+ DRM_ERROR("invalid input\n");
+ return;
+ }
+
+ catalog = container_of(dp_catalog, struct dp_catalog_private,
dp_catalog);
+
+ val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB0) << HEADER_BYTE_0_BIT |
+ (msm_dp_sdp->pb.PB0 << PARITY_BYTE_0_BIT) |
+ (msm_dp_sdp->vsc_sdp.sdp_header.HB1) << HEADER_BYTE_1_BIT |
+ (msm_dp_sdp->pb.PB1 << PARITY_BYTE_1_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_0, val);
+
+ val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB2) << HEADER_BYTE_2_BIT |
+ (msm_dp_sdp->pb.PB2 << PARITY_BYTE_2_BIT) |
+ (msm_dp_sdp->vsc_sdp.sdp_header.HB3) << HEADER_BYTE_3_BIT |
+ (msm_dp_sdp->pb.PB3 << PARITY_BYTE_3_BIT));
+ dp_write_link(catalog, MMSS_DP_GENERIC0_1, val);
I still think that this is not the way to do it. Could you please
extract the function that takes struct dp_sdp_header, calculates
padding and writes resulting data to the hardware? This way we can
reuse it later for all the dp_audio stuff.
hmmm ... dp_utils_pack_sdp_header() does that you are asking for right?
OR are you asking for another function like:
1) rename dp_utils_pack_sdp_header() to dp_utils_calc_sdp_parity()
2) dp_utils_pack_sdp() takes two u32 to pack the header and parity
together and we move the << HEADER_BYTE_xx | part to it
dp_catalog_panel_send_vsc_sdp() just uses these two u32 to write the
headers.
I'm really looking for the following function:
void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog,
struct dp_sdp *dp_sdp)
{
dp_write_vsc_header(dp_catalog, MMSS_DP_GENERIC0_0, &dp_sdp->sdp_header);
dp_write_vsc_packet(dp_catalog, MMSS_DP_GENERIC0_2, dp_sdp);
}
Then dp_audio functions will be able to fill struct dp_sdp_header and
call dp_write_vsc_header (or whatever other name for that function)
directly.
I think there is some misunderstanding here.
Audio does not write or use generic_0 registers. It uses audio infoframe
SDP regis
Re: [PATCH v3 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP
On Thu, 15 Feb 2024 at 18:39, Abhinav Kumar wrote:
>
>
>
> On 2/15/2024 12:40 AM, Dmitry Baryshkov wrote:
> > On Wed, 14 Feb 2024 at 22:15, Abhinav Kumar
> > wrote:
> >>
> >>
> >>
> >> On 2/14/2024 11:39 AM, Dmitry Baryshkov wrote:
> >>> On Wed, 14 Feb 2024 at 20:04, 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.
>
> Changes in v3:
> - Create a new struct, msm_dp_sdp_with_parity, which holds the
> packing information for VSC SDP
> - Use drm_dp_vsc_sdp_pack() to pack the data into the new
> msm_dp_sdp_with_parity struct instead of specifically packing
> for YUV420 format
> - Modify dp_catalog_panel_send_vsc_sdp() to send the VSC SDP
> data using the new msm_dp_sdp_with_parity struct
>
> Changes in v2:
> - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
> - Remove dp_sdp from the dp_catalog struct since this data is
> being allocated at the point used
> - Create a new function in dp_utils to pack the VSC SDP data
> into a buffer
> - Create a new function that packs the SDP header bytes into a
> buffer. This function is made generic so that it can be
> utilized by dp_audio
> header bytes into a buffer
> - Create a new function in dp_utils that takes the packed
> buffer
> and writes to the DP_GENERIC0_* registers
> - Split the dp_catalog_panel_config_vsc_sdp() function into two
> to disable/enable sending VSC SDP packets
> - Check the DP HW version using the original useage of
> dp_catalog_hw_revision() and correct the version checking
> logic
> - Rename dp_panel_setup_vsc_sdp() to
> dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
> currently VSC SDP is only being set up to support YUV420
> modes
>
> Signed-off-by: Paloma Arellano
> ---
> drivers/gpu/drm/msm/dp/dp_catalog.c | 113
> drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++
> drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
> drivers/gpu/drm/msm/dp/dp_panel.c | 55 ++
> drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
> drivers/gpu/drm/msm/dp/dp_utils.c | 48
> drivers/gpu/drm/msm/dp/dp_utils.h | 18 +
> 7 files changed, 248 insertions(+)
>
> diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> b/drivers/gpu/drm/msm/dp/dp_catalog.c
> index 5d84c089e520a..61d5317efe683 100644
> --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> @@ -901,6 +901,119 @@ int dp_catalog_panel_timing_cfg(struct dp_catalog
> *dp_catalog)
> return 0;
> }
>
> +static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog *dp_catalog,
> + struct msm_dp_sdp_with_parity
> *msm_dp_sdp)
> +{
> + struct dp_catalog_private *catalog;
> + u32 val;
> +
> + if (!dp_catalog) {
> + DRM_ERROR("invalid input\n");
> + return;
> + }
> +
> + catalog = container_of(dp_catalog, struct dp_catalog_private,
> dp_catalog);
> +
> + val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB0) << HEADER_BYTE_0_BIT
> |
> + (msm_dp_sdp->pb.PB0 << PARITY_BYTE_0_BIT) |
> + (msm_dp_sdp->vsc_sdp.sdp_header.HB1) << HEADER_BYTE_1_BIT
> |
> + (msm_dp_sdp->pb.PB1 << PARITY_BYTE_1_BIT));
> + dp_write_link(catalog, MMSS_DP_GENERIC0_0, val);
> +
> + val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB2) << HEADER_BYTE_2_BIT
> |
> + (msm_dp_sdp->pb.PB2 << PARITY_BYTE_2_BIT) |
> + (msm_dp_sdp->vsc_sdp.sdp_header.HB3) << HEADER_BYTE_3_BIT
> |
> + (msm_dp_sdp->pb.PB3 << PARITY_BYTE_3_BIT));
> + dp_write_link(catalog, MMSS_DP_GENERIC0_1, val);
> >>>
> >>> I still think that this is not the way to do it. Could you please
> >>> extract the function that takes struct dp_sdp_header, calculates
> >>> padding and writes resulting data to the hardware? This way we can
> >>> reuse it later for all the dp_audio stuff.
> >>>
> >>
> >> hmmm ... dp_utils_pack_sdp_header() does that you are asking for right?
> >>
> >> OR are you asking for another function like:
> >>
> >> 1) rename dp_utils_pack_sdp_header() to dp_utils_calc_sdp_parity()
> >>
Re: [PATCH v3 13/19] drm/msm/dp: add VSC SDP support for YUV420 over DP
On Thu, 15 Feb 2024 at 19:03, Dmitry Baryshkov
wrote:
>
> On Thu, 15 Feb 2024 at 18:39, Abhinav Kumar wrote:
> >
> >
> >
> > On 2/15/2024 12:40 AM, Dmitry Baryshkov wrote:
> > > On Wed, 14 Feb 2024 at 22:15, Abhinav Kumar
> > > wrote:
> > >>
> > >>
> > >>
> > >> On 2/14/2024 11:39 AM, Dmitry Baryshkov wrote:
> > >>> On Wed, 14 Feb 2024 at 20:04, 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.
> >
> > Changes in v3:
> > - Create a new struct, msm_dp_sdp_with_parity, which holds
> > the
> > packing information for VSC SDP
> > - Use drm_dp_vsc_sdp_pack() to pack the data into the new
> > msm_dp_sdp_with_parity struct instead of specifically
> > packing
> > for YUV420 format
> > - Modify dp_catalog_panel_send_vsc_sdp() to send the VSC SDP
> > data using the new msm_dp_sdp_with_parity struct
> >
> > Changes in v2:
> > - Rename GENERIC0_SDPSIZE macro to GENERIC0_SDPSIZE_VALID
> > - Remove dp_sdp from the dp_catalog struct since this data is
> > being allocated at the point used
> > - Create a new function in dp_utils to pack the VSC SDP data
> > into a buffer
> > - Create a new function that packs the SDP header bytes into
> > a
> > buffer. This function is made generic so that it can be
> > utilized by dp_audio
> > header bytes into a buffer
> > - Create a new function in dp_utils that takes the packed
> > buffer
> > and writes to the DP_GENERIC0_* registers
> > - Split the dp_catalog_panel_config_vsc_sdp() function into
> > two
> > to disable/enable sending VSC SDP packets
> > - Check the DP HW version using the original useage of
> > dp_catalog_hw_revision() and correct the version checking
> > logic
> > - Rename dp_panel_setup_vsc_sdp() to
> > dp_panel_setup_vsc_sdp_yuv_420() to explicitly state that
> > currently VSC SDP is only being set up to support YUV420
> > modes
> >
> > Signed-off-by: Paloma Arellano
> > ---
> > drivers/gpu/drm/msm/dp/dp_catalog.c | 113
> >
> > drivers/gpu/drm/msm/dp/dp_catalog.h | 7 ++
> > drivers/gpu/drm/msm/dp/dp_ctrl.c| 4 +
> > drivers/gpu/drm/msm/dp/dp_panel.c | 55 ++
> > drivers/gpu/drm/msm/dp/dp_reg.h | 3 +
> > drivers/gpu/drm/msm/dp/dp_utils.c | 48
> > drivers/gpu/drm/msm/dp/dp_utils.h | 18 +
> > 7 files changed, 248 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/dp/dp_catalog.c
> > b/drivers/gpu/drm/msm/dp/dp_catalog.c
> > index 5d84c089e520a..61d5317efe683 100644
> > --- a/drivers/gpu/drm/msm/dp/dp_catalog.c
> > +++ b/drivers/gpu/drm/msm/dp/dp_catalog.c
> > @@ -901,6 +901,119 @@ int dp_catalog_panel_timing_cfg(struct
> > dp_catalog *dp_catalog)
> > return 0;
> > }
> >
> > +static void dp_catalog_panel_send_vsc_sdp(struct dp_catalog
> > *dp_catalog,
> > + struct
> > msm_dp_sdp_with_parity *msm_dp_sdp)
> > +{
> > + struct dp_catalog_private *catalog;
> > + u32 val;
> > +
> > + if (!dp_catalog) {
> > + DRM_ERROR("invalid input\n");
> > + return;
> > + }
> > +
> > + catalog = container_of(dp_catalog, struct dp_catalog_private,
> > dp_catalog);
> > +
> > + val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB0) <<
> > HEADER_BYTE_0_BIT |
> > + (msm_dp_sdp->pb.PB0 << PARITY_BYTE_0_BIT) |
> > + (msm_dp_sdp->vsc_sdp.sdp_header.HB1) <<
> > HEADER_BYTE_1_BIT |
> > + (msm_dp_sdp->pb.PB1 << PARITY_BYTE_1_BIT));
> > + dp_write_link(catalog, MMSS_DP_GENERIC0_0, val);
> > +
> > + val = ((msm_dp_sdp->vsc_sdp.sdp_header.HB2) <<
> > HEADER_BYTE_2_BIT |
> > + (msm_dp_sdp->pb.PB2 << PARITY_BYTE_2_BIT) |
> > + (msm_dp_sdp->vsc_sdp.sdp_header.HB3) <<
> > HEADER_BYTE_3_BIT |
> > + (msm_dp_sdp->pb.PB3 << PARITY_BYTE_3_BIT));
> > + dp_write_link(catalog, MMSS_DP_GENERIC0_1, val);
> > >>>
> > >>> I still think that this is not the way to do it. Could you please
> > >>> extract the function that takes struct dp_sdp_header, calculates
> > >>> padding and wr
