On 2/20/2024 10:09 AM, Dmitry Baryshkov wrote:
On Tue, 20 Feb 2024 at 19:55, Paloma Arellano <quic_parellan@xxxxxxxxxxx> wrote:
On 2/17/2024 12:56 AM, Dmitry Baryshkov wrote:
On Sat, 17 Feb 2024 at 01:03, Paloma Arellano <quic_parellan@xxxxxxxxxxx> wrote:
+ }
+
+ panel = container_of(dp_panel, struct dp_panel_private, dp_panel);
+ catalog = panel->catalog;
+ dp_mode = &dp_panel->dp_mode;
+
+ memset(&vsc_sdp_data, 0, sizeof(vsc_sdp_data));
+
+ /* VSC SDP header as per table 2-118 of DP 1.4 specification */
+ vsc_sdp_data.sdp_type = DP_SDP_VSC;
+ vsc_sdp_data.revision = 0x05;
+ vsc_sdp_data.length = 0x13;
+
+ /* VSC SDP Payload for DB16 */
+ vsc_sdp_data.pixelformat = DP_PIXELFORMAT_YUV420;
+ vsc_sdp_data.colorimetry = DP_COLORIMETRY_DEFAULT;
+
+ /* VSC SDP Payload for DB17 */
+ vsc_sdp_data.bpc = dp_mode->bpp / 3;
+ vsc_sdp_data.dynamic_range = DP_DYNAMIC_RANGE_CTA;
+
+ /* VSC SDP Payload for DB18 */
+ vsc_sdp_data.content_type = DP_CONTENT_TYPE_GRAPHICS;
+
+ len = dp_utils_pack_vsc_sdp(&vsc_sdp_data, &vsc_sdp, header);
+ if (len < 0) {
+ DRM_ERROR("unable to pack vsc sdp\n");
+ return len;
+ }
So, at this point we have the header data both in vsc_sdp.sdp_header
and in the packed header. The relationship between them becomes less
obvious. Could you please pack dp_sdp_header into u32[2] just before
writing it? It really makes little sense to pass both at the same
time.
Just want to clear some stuff up. Do you want to call the
dp_utils_pack_sdp_header() function right before
dp_catalog_panel_send_vsc_sdp()? The point of putting the
dp_utils_pack_sdp_header() function inside dp_utils_pack_vsc_sdp() is so
that all of the packing could be in the same location. Although I agree
that we are passing the same values twice, I believe that having it the
way it is currently is better since it shows that the
drm_dp_vsc_sdp_pack() and dp_utils_pack_sdp_header() are related since
they're packing the data to the set of GENERIC0 registers.
I'm perfectly fine with dp_utils_pack_sdp_header() being called from
within dp_catalog_panel_send_vsc_sdp(). This way you are not passing
extra data and it is perfectly clear how the SDP header is handled
before being written to the hardware.
Ack. Sounds good, I'll implement it that way
