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.
--
With best wishes
Dmitry
