mbox series

[v17,0/9] Enable Adaptive Sync SDP Support for DP

Message ID 20240311094238.3320888-1-mitulkumar.ajitkumar.golani@intel.com (mailing list archive)
Headers show
Series Enable Adaptive Sync SDP Support for DP | expand

Message

Golani, Mitulkumar Ajitkumar March 11, 2024, 9:42 a.m. UTC
An Adaptive-Sync-capable DP protocol converter indicates its
support by setting the related bit in the DPCD register. This
is valid for DP and edp as well.

Computes AS SDP values based on the display configuration,
ensuring proper handling of Variable Refresh Rate (VRR)
in the context of Adaptive Sync.

--v2:
- Update logging to Patch-1
- use as_sdp instead of async
- Put definitions to correct placeholders from where it is defined.
- Update member types of as_sdp for uniformity.
- Correct use of REG_BIT and REG_GENMASK.
- Remove unrelated comments and changes.
- Correct code indents.
- separate out patch changes for intel_read/write_dp_sdp.

--v3:
- Add VIDEO_DIP_ASYNC_DATA_SIZE definition and comment in as_sdp_pack
  function to patch 2 as originally used there. [Patch 2].
- Add VIDEO_DIP_ENABLE_AS_HSW flag to intel_dp_set_infoframes [Patch 3].

--v4:
- Add check for HAS_VRR before writing AS SDP. [Patch 3].

--v5:
- Add missing check for HAS_VRR before reading AS SDP as well [Patch 3].

--v6:
- Rebase all patches.
- Compute TRANS_VRR_VSYNC.

-v7:
- Move vrr_vsync_start/end to compute config.
- Use correct function for drm_debug_printer.

-v8:
- Code refactoring.
- Update, VSYNC_START/END macros to VRR_VSYNC_START/END.(Ankit)
- Update bit fields of VRR_VSYNC_START/END.(Ankit)
- Send patches to dri-devel.(Ankit)
- Update definition names for AS SDP which are starting from
HSW, as these defines are applicable for ADLP+.(Ankit)
- Remove unused bitfield define, AS_SDP_ENABLE.
- Add support in drm for Adaptive Sync sink status, which can be
used later as a check for read/write sdp. (Ankit)

-v9:
- Add enum to operation mode to represent different AVT and
FAVT modes. (Ankit)
- Operation_mode, target_rr etc should be filled from as_sdp struct. (Ankit)
- Fill as_sdp->*All Params* from compute config, read from the sdp. (Ankit)
- Move configs to the appropriate patch where it used first.(Ankit)
- There should be a check if as sdp is enable is set or not. (Ankit)
- Add variables in crtc state->vrr for ad sdp enable and operation mode. (Ankit)
- Use above variables for tracking AS SDP. (Ankit)
- Revert unused changes. (Ankit)

-v10:
- Send Patches to dri-devel (Ankit).

-v11:
- Remove as_sdp_mode and enable from crtc_state.
- For consistency, update ADL_ prefix or post fix as required.
- Add a comment mentioning current support of
  DP_AS_SDP_AVT_FIXED_VTOTAL.
- Add state checker for AS_SDP infoframe enable.
- Add PIPE_CONF_CHECK_I(vrr.vsync_start/end).
- Read/write vrr_vsync params only when we intend to send
adaptive_sync sdp.

-v12:
- Update cover letter

-v13:
- Add correct place holder and name change for AS_SDP_OP_MODE.
- Separate i915 changes from drm changes.
- Remove extra lines.
- Check if AS_SDP bit is set in crtc_state->infoframes.enable. If not
  return.
- Check for HAS_AS_SDP() before setting VIDEO_DIP_ENABLE_AS_ADL mask.
- Just use drm/i915/dp in subject line.
- Drop conn_state from intel_dp_compute_as_sdp, as not used.
- Remove fullstop in subject line.
- crtc_state->infoframes.enable, to add on correct place holder.

--v14:
- Mistakenly dropped first patch, adding back.

--v15:
- Rename intel_read_dp_infoframe_as_sdp to intel_read_dp_as_sdp.
- Add an entry in g4x_infoframe_enable.
- Instead of intel_vrr_is_in_range, use crtc_state->vrr.enable in AS SDP
compute config.

--v16:
- Add drm/i915/display in subject line.
- Use as_sdp->target_rr & 0xFF.
- Shift by 8 instead of 32, and drop casting to u64.
- Remove does not belong to respective patch.
- Remove vrefresh and connector, as they are no longer required.
- Use VRR_SYNC_START/END macros correctly.
- Update commit message for Patch#9

--v17:
- Relocate vrr vsync params.

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>

Mitul Golani (9):
  drm/dp: Add support to indicate if sink supports AS SDP
  drm: Add Adaptive Sync SDP logging
  drm/i915/display: Add crtc state dump for Adaptive Sync SDP
  drm/i915/dp: Add Read/Write support for Adaptive Sync SDP
  drm/i915/dp: Add wrapper function to check AS SDP
  drm/i915/display: Compute AS SDP parameters
  drm/i915/display: Add state checker for Adaptive Sync SDP
  drm/i915/display: Compute vrr_vsync params
  drm/i915/display: Read/Write Adaptive Sync SDP

 drivers/gpu/drm/display/drm_dp_helper.c       |  37 ++++++
 .../drm/i915/display/intel_crtc_state_dump.c  |  13 ++
 drivers/gpu/drm/i915/display/intel_ddi.c      |   1 +
 drivers/gpu/drm/i915/display/intel_display.c  |  48 +++++++
 .../drm/i915/display/intel_display_device.h   |   1 +
 .../drm/i915/display/intel_display_types.h    |   2 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 125 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.h       |   1 +
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  14 +-
 drivers/gpu/drm/i915/display/intel_vrr.c      |  30 ++++-
 drivers/gpu/drm/i915/i915_reg.h               |  15 +++
 include/drm/display/drm_dp.h                  |  11 ++
 include/drm/display/drm_dp_helper.h           |  30 +++++
 13 files changed, 325 insertions(+), 3 deletions(-)

Comments

Jani Nikula March 18, 2024, 2:37 p.m. UTC | #1
On Mon, 11 Mar 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
>  An Adaptive-Sync-capable DP protocol converter indicates its
> support by setting the related bit in the DPCD register. This
> is valid for DP and edp as well.
>
> Computes AS SDP values based on the display configuration,
> ensuring proper handling of Variable Refresh Rate (VRR)
> in the context of Adaptive Sync.

[snip]

> Mitul Golani (9):
>   drm/dp: Add support to indicate if sink supports AS SDP
>   drm: Add Adaptive Sync SDP logging

Maarten, Maxime, Thomas, ack for merging these two patches via
drm-intel-next?

BR,
Jani.

>   drm/i915/display: Add crtc state dump for Adaptive Sync SDP
>   drm/i915/dp: Add Read/Write support for Adaptive Sync SDP
>   drm/i915/dp: Add wrapper function to check AS SDP
>   drm/i915/display: Compute AS SDP parameters
>   drm/i915/display: Add state checker for Adaptive Sync SDP
>   drm/i915/display: Compute vrr_vsync params
>   drm/i915/display: Read/Write Adaptive Sync SDP
>
>  drivers/gpu/drm/display/drm_dp_helper.c       |  37 ++++++
>  .../drm/i915/display/intel_crtc_state_dump.c  |  13 ++
>  drivers/gpu/drm/i915/display/intel_ddi.c      |   1 +
>  drivers/gpu/drm/i915/display/intel_display.c  |  48 +++++++
>  .../drm/i915/display/intel_display_device.h   |   1 +
>  .../drm/i915/display/intel_display_types.h    |   2 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 125 ++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dp.h       |   1 +
>  drivers/gpu/drm/i915/display/intel_hdmi.c     |  14 +-
>  drivers/gpu/drm/i915/display/intel_vrr.c      |  30 ++++-
>  drivers/gpu/drm/i915/i915_reg.h               |  15 +++
>  include/drm/display/drm_dp.h                  |  11 ++
>  include/drm/display/drm_dp_helper.h           |  30 +++++
>  13 files changed, 325 insertions(+), 3 deletions(-)
Maxime Ripard March 19, 2024, 9:46 a.m. UTC | #2
On Mon, Mar 18, 2024 at 04:37:58PM +0200, Jani Nikula wrote:
> On Mon, 11 Mar 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> >  An Adaptive-Sync-capable DP protocol converter indicates its
> > support by setting the related bit in the DPCD register. This
> > is valid for DP and edp as well.
> >
> > Computes AS SDP values based on the display configuration,
> > ensuring proper handling of Variable Refresh Rate (VRR)
> > in the context of Adaptive Sync.
> 
> [snip]
> 
> > Mitul Golani (9):
> >   drm/dp: Add support to indicate if sink supports AS SDP
> >   drm: Add Adaptive Sync SDP logging
> 
> Maarten, Maxime, Thomas, ack for merging these two patches via
> drm-intel-next?

Ack

Maxime
Ankit Nautiyal April 4, 2024, 2:59 p.m. UTC | #3
On 3/19/2024 3:16 PM, Maxime Ripard wrote:
> On Mon, Mar 18, 2024 at 04:37:58PM +0200, Jani Nikula wrote:
>> On Mon, 11 Mar 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
>>>   An Adaptive-Sync-capable DP protocol converter indicates its
>>> support by setting the related bit in the DPCD register. This
>>> is valid for DP and edp as well.
>>>
>>> Computes AS SDP values based on the display configuration,
>>> ensuring proper handling of Variable Refresh Rate (VRR)
>>> in the context of Adaptive Sync.
>> [snip]
>>
>>> Mitul Golani (9):
>>>    drm/dp: Add support to indicate if sink supports AS SDP
>>>    drm: Add Adaptive Sync SDP logging
>> Maarten, Maxime, Thomas, ack for merging these two patches via
>> drm-intel-next?
> Ack
>
> Maxime

Thanks for the patch, ack and reviews, pushed to drm-intel-next.

Regards,

Ankit
Jani Nikula April 19, 2024, 12:35 p.m. UTC | #4
On Thu, 04 Apr 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> On 3/19/2024 3:16 PM, Maxime Ripard wrote:
>> On Mon, Mar 18, 2024 at 04:37:58PM +0200, Jani Nikula wrote:
>>> On Mon, 11 Mar 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
>>>>   An Adaptive-Sync-capable DP protocol converter indicates its
>>>> support by setting the related bit in the DPCD register. This
>>>> is valid for DP and edp as well.
>>>>
>>>> Computes AS SDP values based on the display configuration,
>>>> ensuring proper handling of Variable Refresh Rate (VRR)
>>>> in the context of Adaptive Sync.
>>> [snip]
>>>
>>>> Mitul Golani (9):
>>>>    drm/dp: Add support to indicate if sink supports AS SDP
>>>>    drm: Add Adaptive Sync SDP logging
>>> Maarten, Maxime, Thomas, ack for merging these two patches via
>>> drm-intel-next?
>> Ack
>>
>> Maxime
>
> Thanks for the patch, ack and reviews, pushed to drm-intel-next.

This came up again today [1]. The acks absolutely must be recorded in
the commit messages when pushing the patches.

dim should complain about applying non-i915 patches without acks.

BR,
Jani.


[1] https://lore.kernel.org/r/Zh_Q72gYKMMbge9A@intel.com
Maxime Ripard April 19, 2024, 12:51 p.m. UTC | #5
On Fri, Apr 19, 2024 at 03:35:55PM +0300, Jani Nikula wrote:
> On Thu, 04 Apr 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> > On 3/19/2024 3:16 PM, Maxime Ripard wrote:
> >> On Mon, Mar 18, 2024 at 04:37:58PM +0200, Jani Nikula wrote:
> >>> On Mon, 11 Mar 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> >>>>   An Adaptive-Sync-capable DP protocol converter indicates its
> >>>> support by setting the related bit in the DPCD register. This
> >>>> is valid for DP and edp as well.
> >>>>
> >>>> Computes AS SDP values based on the display configuration,
> >>>> ensuring proper handling of Variable Refresh Rate (VRR)
> >>>> in the context of Adaptive Sync.
> >>> [snip]
> >>>
> >>>> Mitul Golani (9):
> >>>>    drm/dp: Add support to indicate if sink supports AS SDP
> >>>>    drm: Add Adaptive Sync SDP logging
> >>> Maarten, Maxime, Thomas, ack for merging these two patches via
> >>> drm-intel-next?
> >> Ack
> >>
> >> Maxime
> >
> > Thanks for the patch, ack and reviews, pushed to drm-intel-next.
> 
> This came up again today [1]. The acks absolutely must be recorded in
> the commit messages when pushing the patches.
> 
> dim should complain about applying non-i915 patches without acks.

It doesn't at the moment, this has bitten us a couple of times in
drm-misc too.

I did a MR to address that that hasn't been reviewed yet:
https://gitlab.freedesktop.org/drm/maintainer-tools/-/merge_requests/40

Maxime
Jani Nikula April 19, 2024, 2:09 p.m. UTC | #6
On Fri, 19 Apr 2024, Maxime Ripard <mripard@kernel.org> wrote:
> On Fri, Apr 19, 2024 at 03:35:55PM +0300, Jani Nikula wrote:
>> On Thu, 04 Apr 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
>> > On 3/19/2024 3:16 PM, Maxime Ripard wrote:
>> >> On Mon, Mar 18, 2024 at 04:37:58PM +0200, Jani Nikula wrote:
>> >>> On Mon, 11 Mar 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
>> >>>>   An Adaptive-Sync-capable DP protocol converter indicates its
>> >>>> support by setting the related bit in the DPCD register. This
>> >>>> is valid for DP and edp as well.
>> >>>>
>> >>>> Computes AS SDP values based on the display configuration,
>> >>>> ensuring proper handling of Variable Refresh Rate (VRR)
>> >>>> in the context of Adaptive Sync.
>> >>> [snip]
>> >>>
>> >>>> Mitul Golani (9):
>> >>>>    drm/dp: Add support to indicate if sink supports AS SDP
>> >>>>    drm: Add Adaptive Sync SDP logging
>> >>> Maarten, Maxime, Thomas, ack for merging these two patches via
>> >>> drm-intel-next?
>> >> Ack
>> >>
>> >> Maxime
>> >
>> > Thanks for the patch, ack and reviews, pushed to drm-intel-next.
>> 
>> This came up again today [1]. The acks absolutely must be recorded in
>> the commit messages when pushing the patches.
>> 
>> dim should complain about applying non-i915 patches without acks.
>
> It doesn't at the moment, this has bitten us a couple of times in
> drm-misc too.

See check_maintainer() that gets called from apply_patch() and
dim_b4_shazam_branch().

It's of limited value, but it should complain while applying non-i915
patches.

> I did a MR to address that that hasn't been reviewed yet:
> https://gitlab.freedesktop.org/drm/maintainer-tools/-/merge_requests/40

Yeah, a more generic solution is needed, but I think we should unify
with the above.

BR,
Jani.
Maxime Ripard April 19, 2024, 2:33 p.m. UTC | #7
On Fri, Apr 19, 2024 at 05:09:29PM +0300, Jani Nikula wrote:
> On Fri, 19 Apr 2024, Maxime Ripard <mripard@kernel.org> wrote:
> > On Fri, Apr 19, 2024 at 03:35:55PM +0300, Jani Nikula wrote:
> >> On Thu, 04 Apr 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
> >> > On 3/19/2024 3:16 PM, Maxime Ripard wrote:
> >> >> On Mon, Mar 18, 2024 at 04:37:58PM +0200, Jani Nikula wrote:
> >> >>> On Mon, 11 Mar 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> >> >>>>   An Adaptive-Sync-capable DP protocol converter indicates its
> >> >>>> support by setting the related bit in the DPCD register. This
> >> >>>> is valid for DP and edp as well.
> >> >>>>
> >> >>>> Computes AS SDP values based on the display configuration,
> >> >>>> ensuring proper handling of Variable Refresh Rate (VRR)
> >> >>>> in the context of Adaptive Sync.
> >> >>> [snip]
> >> >>>
> >> >>>> Mitul Golani (9):
> >> >>>>    drm/dp: Add support to indicate if sink supports AS SDP
> >> >>>>    drm: Add Adaptive Sync SDP logging
> >> >>> Maarten, Maxime, Thomas, ack for merging these two patches via
> >> >>> drm-intel-next?
> >> >> Ack
> >> >>
> >> >> Maxime
> >> >
> >> > Thanks for the patch, ack and reviews, pushed to drm-intel-next.
> >> 
> >> This came up again today [1]. The acks absolutely must be recorded in
> >> the commit messages when pushing the patches.
> >> 
> >> dim should complain about applying non-i915 patches without acks.
> >
> > It doesn't at the moment, this has bitten us a couple of times in
> > drm-misc too.
> 
> See check_maintainer() that gets called from apply_patch() and
> dim_b4_shazam_branch().

Oh, right, I missed it sorry.

> It's of limited value, but it should complain while applying non-i915
> patches.
> 
> > I did a MR to address that that hasn't been reviewed yet:
> > https://gitlab.freedesktop.org/drm/maintainer-tools/-/merge_requests/40
> 
> Yeah, a more generic solution is needed, but I think we should unify
> with the above.

Fully agreed, I'll rework it.

Thanks!
Maxime
Ankit Nautiyal April 21, 2024, 4:11 a.m. UTC | #8
On 4/19/2024 6:05 PM, Jani Nikula wrote:
> On Thu, 04 Apr 2024, "Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com> wrote:
>> On 3/19/2024 3:16 PM, Maxime Ripard wrote:
>>> On Mon, Mar 18, 2024 at 04:37:58PM +0200, Jani Nikula wrote:
>>>> On Mon, 11 Mar 2024, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
>>>>>    An Adaptive-Sync-capable DP protocol converter indicates its
>>>>> support by setting the related bit in the DPCD register. This
>>>>> is valid for DP and edp as well.
>>>>>
>>>>> Computes AS SDP values based on the display configuration,
>>>>> ensuring proper handling of Variable Refresh Rate (VRR)
>>>>> in the context of Adaptive Sync.
>>>> [snip]
>>>>
>>>>> Mitul Golani (9):
>>>>>     drm/dp: Add support to indicate if sink supports AS SDP
>>>>>     drm: Add Adaptive Sync SDP logging
>>>> Maarten, Maxime, Thomas, ack for merging these two patches via
>>>> drm-intel-next?
>>> Ack
>>>
>>> Maxime
>> Thanks for the patch, ack and reviews, pushed to drm-intel-next.
> This came up again today [1]. The acks absolutely must be recorded in
> the commit messages when pushing the patches.

I apologize for the oversight. Moving forward, I will ensure to 
consistently include the "acked-by" tag when pushing such changes.

Regards,

Ankit

>
> dim should complain about applying non-i915 patches without acks.
>
> BR,
> Jani.
>
>
> [1] https://lore.kernel.org/r/Zh_Q72gYKMMbge9A@intel.com
>
>