mbox series

[v2,00/20] drm: Use new DRM printk funcs (like drm_dbg_*()) in DP helpers

Message ID 20210326203807.105754-1-lyude@redhat.com (mailing list archive)
Headers show
Series drm: Use new DRM printk funcs (like drm_dbg_*()) in DP helpers | expand

Message

Lyude Paul March 26, 2021, 8:37 p.m. UTC
Since it's been asked quite a few times on some of the various DP
related patch series I've submitted to use the new DRM printk helpers,
and it technically wasn't really trivial to do this before due to the
lack of a consistent way to find a drm_device for an AUX channel, this
patch series aims to address this. In this series we:

* Clean-up potentially erroneous usages of drm_dp_aux_init() and
  drm_dp_aux_register() so that actual AUX registration doesn't happen
  until we have an associated DRM device
* Clean-up any obvious errors in drivers we find along the way
* Add a backpointer to the respective drm_device for an AUX channel in
  drm_dp_aux.drm_dev, and hook it up in every driver with an AUX channel
  across the tree
* Add a new ratelimited print helper we'll need for converting the DP
  helpers over to using the new DRM printk helpers
* Fix any inconsistencies with logging in drm_dp_helper.c so we always
  have the aux channel name printed
* Prepare the various DP helpers so they can find the correct drm_device
  to use for logging
* And finally, convert all of the DP helpers over to using drm_dbg_*()
  and drm_err().

Series-wide changes in v2:
* Address most checkpatch issues ('most' as in all except for one line
  going two chars over 100 in "drm/dp_mst: Pass drm_dp_mst_topology_mgr
  to drm_dp_get_vc_payload_bw()" as this was the style in use
  previously, and 2 chars over the limit looks nicer then trying to
  line-wrap this
* Don't rewrap comments

Lyude Paul (20):
  drm/dp: Fixup kernel docs for struct drm_dp_aux
  drm/tegra: Don't register DP AUX channels before connectors
  drm/bridge/cdns-mhdp8546: Register DP aux channel with userspace
  drm/nouveau/kms/nv50-: Move AUX adapter reg to connector late
    register/early unregister
  drm/dp: Add backpointer to drm_device in drm_dp_aux
  drm/dp: Clarify DP AUX registration time
  drm/print: Fixup DRM_DEBUG_KMS_RATELIMITED()
  drm/dp: Pass drm_dp_aux to drm_dp_link_train_clock_recovery_delay()
  drm/dp: Pass drm_dp_aux to drm_dp*_link_train_channel_eq_delay()
  drm/dp: Always print aux channel name in logs
  drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_detect()
  drm/dp_dual_mode: Pass drm_device to
    drm_dp_dual_mode_set_tmds_output()
  drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_max_tmds_clock()
  drm/dp_dual_mode: Pass drm_device to
    drm_dp_dual_mode_get_tmds_output()
  drm/dp_dual_mode: Pass drm_device to drm_lspcon_(get|set)_mode()
  drm/dp_mst: Pass drm_dp_mst_topology_mgr to drm_dp_get_vc_payload_bw()
  drm/dp: Convert drm_dp_helper.c to using drm_err/drm_dbg_*()
  drm/dp_dual_mode: Convert drm_dp_dual_mode_helper.c to using
    drm_err/drm_dbg_kms()
  drm/dp_mst: Drop DRM_ERROR() on kzalloc() fail in
    drm_dp_mst_handle_up_req()
  drm/dp_mst: Convert drm_dp_mst_topology.c to drm_err()/drm_dbg*()

 drivers/gpu/drm/amd/amdgpu/atombios_dp.c      |   5 +-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |   1 +
 .../drm/bridge/analogix/analogix-anx6345.c    |   1 +
 .../drm/bridge/analogix/analogix-anx78xx.c    |   1 +
 .../drm/bridge/analogix/analogix_dp_core.c    |   1 +
 .../drm/bridge/cadence/cdns-mhdp8546-core.c   |  12 +-
 drivers/gpu/drm/bridge/tc358767.c             |   1 +
 drivers/gpu/drm/bridge/ti-sn65dsi86.c         |   1 +
 drivers/gpu/drm/drm_dp_aux_dev.c              |   6 +
 drivers/gpu/drm/drm_dp_dual_mode_helper.c     |  68 ++--
 drivers/gpu/drm/drm_dp_helper.c               | 181 +++++----
 drivers/gpu/drm/drm_dp_mst_topology.c         | 381 +++++++++---------
 drivers/gpu/drm/i915/display/intel_dp_aux.c   |   1 +
 .../drm/i915/display/intel_dp_link_training.c |   6 +-
 drivers/gpu/drm/i915/display/intel_dp_mst.c   |   3 +-
 drivers/gpu/drm/i915/display/intel_hdmi.c     |   7 +-
 drivers/gpu/drm/i915/display/intel_lspcon.c   |  17 +-
 drivers/gpu/drm/msm/dp/dp_ctrl.c              |   6 +-
 drivers/gpu/drm/msm/edp/edp.h                 |   3 +-
 drivers/gpu/drm/msm/edp/edp_aux.c             |   5 +-
 drivers/gpu/drm/msm/edp/edp_ctrl.c            |   8 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c   |  27 +-
 drivers/gpu/drm/radeon/atombios_dp.c          |   5 +-
 drivers/gpu/drm/tegra/dpaux.c                 |  12 +-
 drivers/gpu/drm/xlnx/zynqmp_dp.c              |   5 +-
 include/drm/drm_dp_dual_mode_helper.h         |  14 +-
 include/drm/drm_dp_helper.h                   |  61 +--
 include/drm/drm_dp_mst_helper.h               |   3 +-
 include/drm/drm_print.h                       |  20 +-
 29 files changed, 478 insertions(+), 384 deletions(-)

Comments

Jani Nikula April 1, 2021, 1:40 p.m. UTC | #1
On Fri, 26 Mar 2021, Lyude Paul <lyude@redhat.com> wrote:
> Since it's been asked quite a few times on some of the various DP
> related patch series I've submitted to use the new DRM printk helpers,
> and it technically wasn't really trivial to do this before due to the
> lack of a consistent way to find a drm_device for an AUX channel, this
> patch series aims to address this. In this series we:
>
> * Clean-up potentially erroneous usages of drm_dp_aux_init() and
>   drm_dp_aux_register() so that actual AUX registration doesn't happen
>   until we have an associated DRM device
> * Clean-up any obvious errors in drivers we find along the way
> * Add a backpointer to the respective drm_device for an AUX channel in
>   drm_dp_aux.drm_dev, and hook it up in every driver with an AUX channel
>   across the tree
> * Add a new ratelimited print helper we'll need for converting the DP
>   helpers over to using the new DRM printk helpers
> * Fix any inconsistencies with logging in drm_dp_helper.c so we always
>   have the aux channel name printed
> * Prepare the various DP helpers so they can find the correct drm_device
>   to use for logging
> * And finally, convert all of the DP helpers over to using drm_dbg_*()
>   and drm_err().
>
> Series-wide changes in v2:
> * Address most checkpatch issues ('most' as in all except for one line
>   going two chars over 100 in "drm/dp_mst: Pass drm_dp_mst_topology_mgr
>   to drm_dp_get_vc_payload_bw()" as this was the style in use
>   previously, and 2 chars over the limit looks nicer then trying to
>   line-wrap this
> * Don't rewrap comments

For anything touching i915, and for merging via whichever tree or branch
seems best,

Acked-by: Jani Nikula <jani.nikula@intel.com>

That said, gut feeling says there will be conflicts before latest
drm-misc-next and drm-intel-next have been merged to drm-next, and
drm-next has been backmerged to drm-misc-next and drm-intel-next.

It just might be a good idea to wait for those (as well as other driver
feature pulls) to settle, do a topic branch with a common ancestor
between drm-next and drm-misc-next, apply there, merge the topic branch
to drm-misc-next, and let all drivers merge the topic branch as
needed. Due to the timing, otherwise we might have to carry the
conflicts for quite a while.

BR,
Jani.


>
> Lyude Paul (20):
>   drm/dp: Fixup kernel docs for struct drm_dp_aux
>   drm/tegra: Don't register DP AUX channels before connectors
>   drm/bridge/cdns-mhdp8546: Register DP aux channel with userspace
>   drm/nouveau/kms/nv50-: Move AUX adapter reg to connector late
>     register/early unregister
>   drm/dp: Add backpointer to drm_device in drm_dp_aux
>   drm/dp: Clarify DP AUX registration time
>   drm/print: Fixup DRM_DEBUG_KMS_RATELIMITED()
>   drm/dp: Pass drm_dp_aux to drm_dp_link_train_clock_recovery_delay()
>   drm/dp: Pass drm_dp_aux to drm_dp*_link_train_channel_eq_delay()
>   drm/dp: Always print aux channel name in logs
>   drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_detect()
>   drm/dp_dual_mode: Pass drm_device to
>     drm_dp_dual_mode_set_tmds_output()
>   drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_max_tmds_clock()
>   drm/dp_dual_mode: Pass drm_device to
>     drm_dp_dual_mode_get_tmds_output()
>   drm/dp_dual_mode: Pass drm_device to drm_lspcon_(get|set)_mode()
>   drm/dp_mst: Pass drm_dp_mst_topology_mgr to drm_dp_get_vc_payload_bw()
>   drm/dp: Convert drm_dp_helper.c to using drm_err/drm_dbg_*()
>   drm/dp_dual_mode: Convert drm_dp_dual_mode_helper.c to using
>     drm_err/drm_dbg_kms()
>   drm/dp_mst: Drop DRM_ERROR() on kzalloc() fail in
>     drm_dp_mst_handle_up_req()
>   drm/dp_mst: Convert drm_dp_mst_topology.c to drm_err()/drm_dbg*()
>
>  drivers/gpu/drm/amd/amdgpu/atombios_dp.c      |   5 +-
>  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |   1 +
>  .../drm/bridge/analogix/analogix-anx6345.c    |   1 +
>  .../drm/bridge/analogix/analogix-anx78xx.c    |   1 +
>  .../drm/bridge/analogix/analogix_dp_core.c    |   1 +
>  .../drm/bridge/cadence/cdns-mhdp8546-core.c   |  12 +-
>  drivers/gpu/drm/bridge/tc358767.c             |   1 +
>  drivers/gpu/drm/bridge/ti-sn65dsi86.c         |   1 +
>  drivers/gpu/drm/drm_dp_aux_dev.c              |   6 +
>  drivers/gpu/drm/drm_dp_dual_mode_helper.c     |  68 ++--
>  drivers/gpu/drm/drm_dp_helper.c               | 181 +++++----
>  drivers/gpu/drm/drm_dp_mst_topology.c         | 381 +++++++++---------
>  drivers/gpu/drm/i915/display/intel_dp_aux.c   |   1 +
>  .../drm/i915/display/intel_dp_link_training.c |   6 +-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c   |   3 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c     |   7 +-
>  drivers/gpu/drm/i915/display/intel_lspcon.c   |  17 +-
>  drivers/gpu/drm/msm/dp/dp_ctrl.c              |   6 +-
>  drivers/gpu/drm/msm/edp/edp.h                 |   3 +-
>  drivers/gpu/drm/msm/edp/edp_aux.c             |   5 +-
>  drivers/gpu/drm/msm/edp/edp_ctrl.c            |   8 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |  27 +-
>  drivers/gpu/drm/radeon/atombios_dp.c          |   5 +-
>  drivers/gpu/drm/tegra/dpaux.c                 |  12 +-
>  drivers/gpu/drm/xlnx/zynqmp_dp.c              |   5 +-
>  include/drm/drm_dp_dual_mode_helper.h         |  14 +-
>  include/drm/drm_dp_helper.h                   |  61 +--
>  include/drm/drm_dp_mst_helper.h               |   3 +-
>  include/drm/drm_print.h                       |  20 +-
>  29 files changed, 478 insertions(+), 384 deletions(-)
Daniel Vetter April 8, 2021, 10:49 a.m. UTC | #2
On Thu, Apr 01, 2021 at 04:40:33PM +0300, Jani Nikula wrote:
> On Fri, 26 Mar 2021, Lyude Paul <lyude@redhat.com> wrote:
> > Since it's been asked quite a few times on some of the various DP
> > related patch series I've submitted to use the new DRM printk helpers,
> > and it technically wasn't really trivial to do this before due to the
> > lack of a consistent way to find a drm_device for an AUX channel, this
> > patch series aims to address this. In this series we:
> >
> > * Clean-up potentially erroneous usages of drm_dp_aux_init() and
> >   drm_dp_aux_register() so that actual AUX registration doesn't happen
> >   until we have an associated DRM device
> > * Clean-up any obvious errors in drivers we find along the way
> > * Add a backpointer to the respective drm_device for an AUX channel in
> >   drm_dp_aux.drm_dev, and hook it up in every driver with an AUX channel
> >   across the tree
> > * Add a new ratelimited print helper we'll need for converting the DP
> >   helpers over to using the new DRM printk helpers
> > * Fix any inconsistencies with logging in drm_dp_helper.c so we always
> >   have the aux channel name printed
> > * Prepare the various DP helpers so they can find the correct drm_device
> >   to use for logging
> > * And finally, convert all of the DP helpers over to using drm_dbg_*()
> >   and drm_err().
> >
> > Series-wide changes in v2:
> > * Address most checkpatch issues ('most' as in all except for one line
> >   going two chars over 100 in "drm/dp_mst: Pass drm_dp_mst_topology_mgr
> >   to drm_dp_get_vc_payload_bw()" as this was the style in use
> >   previously, and 2 chars over the limit looks nicer then trying to
> >   line-wrap this
> > * Don't rewrap comments
> 
> For anything touching i915, and for merging via whichever tree or branch
> seems best,
> 
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> 
> That said, gut feeling says there will be conflicts before latest
> drm-misc-next and drm-intel-next have been merged to drm-next, and
> drm-next has been backmerged to drm-misc-next and drm-intel-next.
> 
> It just might be a good idea to wait for those (as well as other driver
> feature pulls) to settle, do a topic branch with a common ancestor
> between drm-next and drm-misc-next, apply there, merge the topic branch
> to drm-misc-next, and let all drivers merge the topic branch as
> needed. Due to the timing, otherwise we might have to carry the
> conflicts for quite a while.

I think Dave caught up on pulls to drm-next, so after a backmerge of that
to drm-misc-next I think should be all fine to apply directly, no need for
topic branch.
-Daniel

> 
> BR,
> Jani.
> 
> 
> >
> > Lyude Paul (20):
> >   drm/dp: Fixup kernel docs for struct drm_dp_aux
> >   drm/tegra: Don't register DP AUX channels before connectors
> >   drm/bridge/cdns-mhdp8546: Register DP aux channel with userspace
> >   drm/nouveau/kms/nv50-: Move AUX adapter reg to connector late
> >     register/early unregister
> >   drm/dp: Add backpointer to drm_device in drm_dp_aux
> >   drm/dp: Clarify DP AUX registration time
> >   drm/print: Fixup DRM_DEBUG_KMS_RATELIMITED()
> >   drm/dp: Pass drm_dp_aux to drm_dp_link_train_clock_recovery_delay()
> >   drm/dp: Pass drm_dp_aux to drm_dp*_link_train_channel_eq_delay()
> >   drm/dp: Always print aux channel name in logs
> >   drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_detect()
> >   drm/dp_dual_mode: Pass drm_device to
> >     drm_dp_dual_mode_set_tmds_output()
> >   drm/dp_dual_mode: Pass drm_device to drm_dp_dual_mode_max_tmds_clock()
> >   drm/dp_dual_mode: Pass drm_device to
> >     drm_dp_dual_mode_get_tmds_output()
> >   drm/dp_dual_mode: Pass drm_device to drm_lspcon_(get|set)_mode()
> >   drm/dp_mst: Pass drm_dp_mst_topology_mgr to drm_dp_get_vc_payload_bw()
> >   drm/dp: Convert drm_dp_helper.c to using drm_err/drm_dbg_*()
> >   drm/dp_dual_mode: Convert drm_dp_dual_mode_helper.c to using
> >     drm_err/drm_dbg_kms()
> >   drm/dp_mst: Drop DRM_ERROR() on kzalloc() fail in
> >     drm_dp_mst_handle_up_req()
> >   drm/dp_mst: Convert drm_dp_mst_topology.c to drm_err()/drm_dbg*()
> >
> >  drivers/gpu/drm/amd/amdgpu/atombios_dp.c      |   5 +-
> >  .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |   1 +
> >  .../drm/bridge/analogix/analogix-anx6345.c    |   1 +
> >  .../drm/bridge/analogix/analogix-anx78xx.c    |   1 +
> >  .../drm/bridge/analogix/analogix_dp_core.c    |   1 +
> >  .../drm/bridge/cadence/cdns-mhdp8546-core.c   |  12 +-
> >  drivers/gpu/drm/bridge/tc358767.c             |   1 +
> >  drivers/gpu/drm/bridge/ti-sn65dsi86.c         |   1 +
> >  drivers/gpu/drm/drm_dp_aux_dev.c              |   6 +
> >  drivers/gpu/drm/drm_dp_dual_mode_helper.c     |  68 ++--
> >  drivers/gpu/drm/drm_dp_helper.c               | 181 +++++----
> >  drivers/gpu/drm/drm_dp_mst_topology.c         | 381 +++++++++---------
> >  drivers/gpu/drm/i915/display/intel_dp_aux.c   |   1 +
> >  .../drm/i915/display/intel_dp_link_training.c |   6 +-
> >  drivers/gpu/drm/i915/display/intel_dp_mst.c   |   3 +-
> >  drivers/gpu/drm/i915/display/intel_hdmi.c     |   7 +-
> >  drivers/gpu/drm/i915/display/intel_lspcon.c   |  17 +-
> >  drivers/gpu/drm/msm/dp/dp_ctrl.c              |   6 +-
> >  drivers/gpu/drm/msm/edp/edp.h                 |   3 +-
> >  drivers/gpu/drm/msm/edp/edp_aux.c             |   5 +-
> >  drivers/gpu/drm/msm/edp/edp_ctrl.c            |   8 +-
> >  drivers/gpu/drm/nouveau/nouveau_connector.c   |  27 +-
> >  drivers/gpu/drm/radeon/atombios_dp.c          |   5 +-
> >  drivers/gpu/drm/tegra/dpaux.c                 |  12 +-
> >  drivers/gpu/drm/xlnx/zynqmp_dp.c              |   5 +-
> >  include/drm/drm_dp_dual_mode_helper.h         |  14 +-
> >  include/drm/drm_dp_helper.h                   |  61 +--
> >  include/drm/drm_dp_mst_helper.h               |   3 +-
> >  include/drm/drm_print.h                       |  20 +-
> >  29 files changed, 478 insertions(+), 384 deletions(-)
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jani Nikula April 8, 2021, 11:13 a.m. UTC | #3
On Thu, 08 Apr 2021, Daniel Vetter <daniel@ffwll.ch> wrote:
> I think Dave caught up on pulls to drm-next, so after a backmerge of that
> to drm-misc-next I think should be all fine to apply directly, no need for
> topic branch.

Yup. We've done the backmerges to drm-intel-next and drm-intel-gt-next,
and are all in sync, it's only the drm-next -> drm-misc-next backmerge
that's still needed.

BR,
Jani.
Lyude Paul April 8, 2021, 6:51 p.m. UTC | #4
JFYI too - there was a legitimate looking CI failure on intel with this series,
so don't be surprised if I have to respin a patch or two (I should be able to
get it asap as I finally just cleared most of the stuff on my plate off for a
while)

On Thu, 2021-04-08 at 14:13 +0300, Jani Nikula wrote:
> On Thu, 08 Apr 2021, Daniel Vetter <daniel@ffwll.ch> wrote:
> > I think Dave caught up on pulls to drm-next, so after a backmerge of that
> > to drm-misc-next I think should be all fine to apply directly, no need for
> > topic branch.
> 
> Yup. We've done the backmerges to drm-intel-next and drm-intel-gt-next,
> and are all in sync, it's only the drm-next -> drm-misc-next backmerge
> that's still needed.
> 
> BR,
> Jani.
>