[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d3674c10-5c29-d917-44f5-758d90d9e679@quicinc.com>
Date: Tue, 20 Feb 2024 10:53:15 -0800
From: Abhinav Kumar <quic_abhinavk@...cinc.com>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
CC: <dri-devel@...ts.freedesktop.org>,
Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Daniel Vetter <daniel@...ll.ch>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Rodrigo Vivi <rodrigo.vivi@...el.com>,
Joonas Lahtinen
<joonas.lahtinen@...ux.intel.com>,
Tvrtko Ursulin
<tvrtko.ursulin@...ux.intel.com>,
<robdclark@...il.com>, <freedreno@...ts.freedesktop.org>,
<intel-gfx@...ts.freedesktop.org>, <ville.syrjala@...ux.intel.com>,
<quic_jesszhan@...cinc.com>, <linux-kernel@...r.kernel.org>,
<intel-xe@...ts.freedesktop.org>
Subject: Re: [PATCH v2] drm/dp: move intel_dp_vsc_sdp_pack() to generic helper
On 2/20/2024 10:49 AM, Dmitry Baryshkov wrote:
> On Thu, 15 Feb 2024 at 21:08, Abhinav Kumar <quic_abhinavk@...cinc.com> wrote:
>>
>> intel_dp_vsc_sdp_pack() can be re-used by other DRM drivers as well.
>> Lets move this to drm_dp_helper to achieve this.
>>
>> changes in v2:
>> - rebased on top of drm-tip
>>
>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
>
> v1 had an explicit comment before the ack:
>
Yes, I remember the comment. I did not make any changes to v2 other than
just rebasing it on drm-tip to get the ack from i915 folks.
>> From my side, with the promise of the size fixup.
>
> However I observe neither a second patch removing the size argument
> nor it being dropped as a part of this patch.
>
Yes, now that in v2 we got the ack for this patch, I can spin a v3 with
the addition of the next patch to remove the size OR as another series
so as to not block the main series which needs this patch.
I would prefer the latter.
>> Signed-off-by: Abhinav Kumar <quic_abhinavk@...cinc.com>
>> ---
>> drivers/gpu/drm/display/drm_dp_helper.c | 78 +++++++++++++++++++++++++
>> drivers/gpu/drm/i915/display/intel_dp.c | 71 +---------------------
>> include/drm/display/drm_dp_helper.h | 3 +
>> 3 files changed, 83 insertions(+), 69 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
>> index 8d6ce46471ae..6c91f400ecb1 100644
>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
>> @@ -2913,6 +2913,84 @@ void drm_dp_vsc_sdp_log(struct drm_printer *p, const struct drm_dp_vsc_sdp *vsc)
>> }
>> EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
>>
>> +/**
>> + * drm_dp_vsc_sdp_pack() - pack a given vsc sdp into generic dp_sdp
>> + * @vsc: vsc sdp initialized according to its purpose as defined in
>> + * table 2-118 - table 2-120 in DP 1.4a specification
>> + * @sdp: valid handle to the generic dp_sdp which will be packed
>> + * @size: valid size of the passed sdp handle
>> + *
>> + * Returns length of sdp on success and error code on failure
>> + */
>> +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
>> + struct dp_sdp *sdp, size_t size)
>> +{
>> + size_t length = sizeof(struct dp_sdp);
>> +
>> + if (size < length)
>> + return -ENOSPC;
>> +
>> + memset(sdp, 0, size);
>> +
>> + /*
>> + * Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
>> + * VSC SDP Header Bytes
>> + */
>> + sdp->sdp_header.HB0 = 0; /* Secondary-Data Packet ID = 0 */
>> + sdp->sdp_header.HB1 = vsc->sdp_type; /* Secondary-data Packet Type */
>> + sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */
>> + sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data Bytes */
>> +
>> + if (vsc->revision == 0x6) {
>> + sdp->db[0] = 1;
>> + sdp->db[3] = 1;
>> + }
>> +
>> + /*
>> + * Revision 0x5 and revision 0x7 supports Pixel Encoding/Colorimetry
>> + * Format as per DP 1.4a spec and DP 2.0 respectively.
>> + */
>> + if (!(vsc->revision == 0x5 || vsc->revision == 0x7))
>> + goto out;
>> +
>> + /* VSC SDP Payload for DB16 through DB18 */
>> + /* Pixel Encoding and Colorimetry Formats */
>> + sdp->db[16] = (vsc->pixelformat & 0xf) << 4; /* DB16[7:4] */
>> + sdp->db[16] |= vsc->colorimetry & 0xf; /* DB16[3:0] */
>> +
>> + switch (vsc->bpc) {
>> + case 6:
>> + /* 6bpc: 0x0 */
>> + break;
>> + case 8:
>> + sdp->db[17] = 0x1; /* DB17[3:0] */
>> + break;
>> + case 10:
>> + sdp->db[17] = 0x2;
>> + break;
>> + case 12:
>> + sdp->db[17] = 0x3;
>> + break;
>> + case 16:
>> + sdp->db[17] = 0x4;
>> + break;
>> + default:
>> + WARN(1, "Missing case %d\n", vsc->bpc);
>> + return -EINVAL;
>> + }
>> +
>> + /* Dynamic Range and Component Bit Depth */
>> + if (vsc->dynamic_range == DP_DYNAMIC_RANGE_CTA)
>> + sdp->db[17] |= 0x80; /* DB17[7] */
>> +
>> + /* Content Type */
>> + sdp->db[18] = vsc->content_type & 0x7;
>> +
>> +out:
>> + return length;
>> +}
>> +EXPORT_SYMBOL(drm_dp_vsc_sdp_pack);
>> +
>> /**
>> * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON
>> * @dpcd: DisplayPort configuration data
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 217196196e50..a9458df475e2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -4089,73 +4089,6 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
>> return false;
>> }
>>
>> -static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
>> - struct dp_sdp *sdp, size_t size)
>> -{
>> - size_t length = sizeof(struct dp_sdp);
>> -
>> - if (size < length)
>> - return -ENOSPC;
>> -
>> - memset(sdp, 0, size);
>> -
>> - /*
>> - * Prepare VSC Header for SU as per DP 1.4a spec, Table 2-119
>> - * VSC SDP Header Bytes
>> - */
>> - sdp->sdp_header.HB0 = 0; /* Secondary-Data Packet ID = 0 */
>> - sdp->sdp_header.HB1 = vsc->sdp_type; /* Secondary-data Packet Type */
>> - sdp->sdp_header.HB2 = vsc->revision; /* Revision Number */
>> - sdp->sdp_header.HB3 = vsc->length; /* Number of Valid Data Bytes */
>> -
>> - if (vsc->revision == 0x6) {
>> - sdp->db[0] = 1;
>> - sdp->db[3] = 1;
>> - }
>> -
>> - /*
>> - * Revision 0x5 and revision 0x7 supports Pixel Encoding/Colorimetry
>> - * Format as per DP 1.4a spec and DP 2.0 respectively.
>> - */
>> - if (!(vsc->revision == 0x5 || vsc->revision == 0x7))
>> - goto out;
>> -
>> - /* VSC SDP Payload for DB16 through DB18 */
>> - /* Pixel Encoding and Colorimetry Formats */
>> - sdp->db[16] = (vsc->pixelformat & 0xf) << 4; /* DB16[7:4] */
>> - sdp->db[16] |= vsc->colorimetry & 0xf; /* DB16[3:0] */
>> -
>> - switch (vsc->bpc) {
>> - case 6:
>> - /* 6bpc: 0x0 */
>> - break;
>> - case 8:
>> - sdp->db[17] = 0x1; /* DB17[3:0] */
>> - break;
>> - case 10:
>> - sdp->db[17] = 0x2;
>> - break;
>> - case 12:
>> - sdp->db[17] = 0x3;
>> - break;
>> - case 16:
>> - sdp->db[17] = 0x4;
>> - break;
>> - default:
>> - MISSING_CASE(vsc->bpc);
>> - break;
>> - }
>> - /* Dynamic Range and Component Bit Depth */
>> - if (vsc->dynamic_range == DP_DYNAMIC_RANGE_CTA)
>> - sdp->db[17] |= 0x80; /* DB17[7] */
>> -
>> - /* Content Type */
>> - sdp->db[18] = vsc->content_type & 0x7;
>> -
>> -out:
>> - return length;
>> -}
>> -
>> static ssize_t
>> intel_dp_hdr_metadata_infoframe_sdp_pack(struct drm_i915_private *i915,
>> const struct hdmi_drm_infoframe *drm_infoframe,
>> @@ -4248,8 +4181,8 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
>>
>> switch (type) {
>> case DP_SDP_VSC:
>> - len = intel_dp_vsc_sdp_pack(&crtc_state->infoframes.vsc, &sdp,
>> - sizeof(sdp));
>> + len = drm_dp_vsc_sdp_pack(&crtc_state->infoframes.vsc, &sdp,
>> + sizeof(sdp));
>> break;
>> case HDMI_PACKET_TYPE_GAMUT_METADATA:
>> len = intel_dp_hdr_metadata_infoframe_sdp_pack(dev_priv,
>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
>> index d02014a87f12..8474504d4c88 100644
>> --- a/include/drm/display/drm_dp_helper.h
>> +++ b/include/drm/display/drm_dp_helper.h
>> @@ -812,4 +812,7 @@ int drm_dp_bw_overhead(int lane_count, int hactive,
>> int bpp_x16, unsigned long flags);
>> int drm_dp_bw_channel_coding_efficiency(bool is_uhbr);
>>
>> +ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
>> + struct dp_sdp *sdp, size_t size);
>> +
>> #endif /* _DRM_DP_HELPER_H_ */
>> --
>> 2.34.1
>>
>
>
Powered by blists - more mailing lists