Re: [PATCH 11/17] drm/msm/dp: add VSC SDP support for YUV420 over DP

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/27/2024 9:39 PM, Dmitry Baryshkov wrote:
On Sun, 28 Jan 2024 at 07:34, Paloma Arellano <quic_parellan@xxxxxxxxxxx> 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 <quic_parellan@xxxxxxxxxxx>
---
   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, &data, 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, &data, 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, &data, 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, &data, sizeof(data));
+    off += sizeof(data);
+
+    dp_write_link(catalog, MMSS_DP_GENERIC0_4, data);
+    memcpy(buf + off, &data, sizeof(data));
+    off += sizeof(data);
+
+    dp_write_link(catalog, MMSS_DP_GENERIC0_5, data);
+    memcpy(buf + off, &data, 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, &data, sizeof(data));
+    off += sizeof(data);
+
+    data = 0;
+    dp_write_link(catalog, MMSS_DP_GENERIC0_7, data);
+    memcpy(buf + off, &data, sizeof(data));
+    off += sizeof(data);
+
+    dp_write_link(catalog, MMSS_DP_GENERIC0_8, data);
+    memcpy(buf + off, &data, sizeof(data));
+    off += sizeof(data);





[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux