dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: Lyude Paul <lyude@redhat.com>
To: Bhawanpreet Lakha <Bhawanpreet.lakha@amd.com>,
	Jerry.Zuo@amd.com,  dri-devel@lists.freedesktop.org
Cc: Harry.Wentland@amd.com, Wayne.Lin@amd.com, Nicholas.Kazlauskas@amd.com
Subject: Re: [PATCH] drm: Update MST First Link Slot Information Based on Encoding Format
Date: Fri, 15 Oct 2021 16:04:24 -0400	[thread overview]
Message-ID: <57b116bf5d90459fac4ce605aeab9b25afca92e6.camel@redhat.com> (raw)
In-Reply-To: <d9a43511-3435-0efc-d8a2-24c035b0ec74@amd.com>

[snip]

On Thu, 2021-10-14 at 16:21 -0400, Bhawanpreet Lakha wrote:
> 
> Thanks for the response,
> 
> That function is per port (drm_dp_atomic_find_vcpi_slots) so not sure 
> how that will work, maybe I don't understand what you mean?

Yeah - drm_dp_atomic_find_vcpi_slots() is just one part of the atomic helpers
though, we can always add more. JFYI, the main atomic check function for MST
is drm_dp_mst_atomic_check(). So we'd probably just want to add something into
drm_dp_mst_topology_state that handles making sure we go through
drm_dp_vcpi_allocation struct and recalculates everything in it. We might also
want to add an atomic helper to set the new starting slot and slot count in
the atomic state, then go through the current atomic topology state and ensure
that we add any CRTCs that would need full modesets as a result of that info
changing.

> 
> Also we only need to check this inside 
> drm_dp_mst_atomic_check_vcpi_alloc_limit(), which doesn't have a state, 
> so we still need to have this on the mgr somehow.

It does, we pass drm_dp_mst_topology_state to it. So we could just add these
fields there.

> 
> I was thinking we could add the slots(or some DP version indicator) 
> inside the drm_connector, and add a parameter to 
> drm_dp_mst_atomic_check_vcpi_alloc_limit(int slots) and call it with 
> this info via drm_dp_mst_atomic_check().

So before I continue: I should note that some of the code in MST goes against
what I'm about to say, in particular a lot of the payload updating functions
in MST that keep their payload state outside of drm_dp_mst_topology_state and
friends, but that's because some of this code is old and needs to be updated
anyway (and some of it was actually just being kept around because we were
worried about breaking radeon.ko, the only driver relying on behavior from the
non-atomic paths in our topology helper). Also - I'm not sure what your prior
experience is with modesetting in the linux kernel so my apologies if I'm
explaining anything you already understand here.

Anyway-drm_connector wouldn't be the right place to put it because it's not
part of the atomic state. The reason we have atomic modesetting in the first
place is so that we can come up with a new display state, and then have the
kernel verify the configurations in that new state and potentially reject it
if we tried to program something that wouldn't have worked in hardware. As
well, having it in drm_connector would mean that it wouldn't be safe to access
unless we've somehow locked the drm_connector. drm_connector_state would be
'safe' to have this data in, but considering that we already have a private
atomic state object for MST (drm_dp_mst_topology_state) it doesn't make much
sense to keep MST info elsewhere.

> 
> 
> Bhawan
> 
> > 
> > >                  ret = drm_dp_mst_atomic_check_mstb_bw_limit(mgr-
> > > > mst_primary,
> > >                                                              mst_state);
> > >                  mutex_unlock(&mgr->lock);
> > > @@ -5527,11 +5543,16 @@ int drm_dp_mst_topology_mgr_init(struct
> > > drm_dp_mst_topology_mgr *mgr,
> > >          if (!mgr->proposed_vcpis)
> > >                  return -ENOMEM;
> > >          set_bit(0, &mgr->payload_mask);
> > > +       mgr->total_avail_slots = 63;
> > > +       mgr->start_slot = 1;
> > >   
> > >          mst_state = kzalloc(sizeof(*mst_state), GFP_KERNEL);
> > >          if (mst_state == NULL)
> > >                  return -ENOMEM;
> > >   
> > > +       mst_state->total_avail_slots = 63;
> > > +       mst_state->start_slot = 1;
> > > +
> > >          mst_state->mgr = mgr;
> > >          INIT_LIST_HEAD(&mst_state->vcpis);
> > >   
> > > diff --git a/include/drm/drm_dp_mst_helper.h
> > > b/include/drm/drm_dp_mst_helper.h
> > > index ddb9231d0309..f8152dfb34ed 100644
> > > --- a/include/drm/drm_dp_mst_helper.h
> > > +++ b/include/drm/drm_dp_mst_helper.h
> > > @@ -554,6 +554,8 @@ struct drm_dp_mst_topology_state {
> > >          struct drm_private_state base;
> > >          struct list_head vcpis;
> > >          struct drm_dp_mst_topology_mgr *mgr;
> > > +       u8 total_avail_slots;
> > > +       u8 start_slot;
> > >   };
> > >   
> > >   #define to_dp_mst_topology_mgr(x) container_of(x, struct
> > > drm_dp_mst_topology_mgr, base)
> > > @@ -661,6 +663,16 @@ struct drm_dp_mst_topology_mgr {
> > >           */
> > >          int pbn_div;
> > >   
> > > +       /**
> > > +        * @total_avail_slots: 63 for 8b/10b, 64 for 128/132b
> > > +        */
> > > +       u8 total_avail_slots;
> > > +
> > > +       /**
> > > +        * @start_slot: 1 for 8b/10b, 0 for 128/132b
> > > +        */
> > > +       u8 start_slot;
> > > +
> > >          /**
> > >           * @funcs: Atomic helper callbacks
> > >           */
> > > @@ -806,6 +818,7 @@ int drm_dp_mst_get_vcpi_slots(struct
> > > drm_dp_mst_topology_mgr *mgr, struct drm_dp
> > >   
> > >   void drm_dp_mst_reset_vcpi_slots(struct drm_dp_mst_topology_mgr *mgr,
> > > struct drm_dp_mst_port *port);
> > >   
> > > +void drm_dp_mst_update_coding_cap(struct drm_dp_mst_topology_state
> > > *mst_state, uint8_t link_coding_cap);
> > >   
> > >   void drm_dp_mst_deallocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
> > >                                  struct drm_dp_mst_port *port);
> 

-- 
Cheers,
 Lyude Paul (she/her)
 Software Engineer at Red Hat


  parent reply	other threads:[~2021-10-15 20:04 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-27 23:43 [PATCH 0/2] Update 128b/132b MST Slot Information Fangzhi Zuo
2021-08-27 23:43 ` [PATCH 1/2] drm: Update MST First Link Slot Information Based on Encoding Format Fangzhi Zuo
2021-08-28  4:09   ` kernel test robot
2021-08-30 20:00   ` Lyude Paul
2021-08-31 19:44     ` Zuo, Jerry
2021-08-31 20:19       ` Lyude Paul
2021-08-31 22:44       ` Lyude Paul
2021-10-12 21:58         ` [PATCH] " Bhawanpreet Lakha
2021-10-12 22:02           ` Lakha, Bhawanpreet
2021-10-12 22:09             ` Lyude Paul
2021-10-13 16:09           ` Jani Nikula
2021-10-13 19:33             ` Bhawanpreet Lakha
2021-10-13 19:37               ` Lyude Paul
2021-10-13 18:52           ` kernel test robot
2021-10-13 21:54           ` kernel test robot
2021-10-13 22:25           ` Lyude Paul
2021-10-14 20:21             ` Bhawanpreet Lakha
2021-10-14 20:22               ` Bhawanpreet Lakha
2021-10-15 20:04               ` Lyude Paul [this message]
2021-10-15 19:43             ` Bhawanpreet Lakha
2021-10-15 20:41               ` Lyude Paul
2021-10-18 19:47                 ` Bhawanpreet Lakha
2021-10-18 23:17                   ` Lyude Paul
2021-10-19 18:01                     ` [PATCH 2/4] " Bhawanpreet Lakha
2021-10-20  8:34                       ` Jani Nikula
2021-08-31 22:45       ` [PATCH 1/2] " Lyude Paul
2021-08-31 22:46         ` Lyude Paul
2021-08-27 23:43 ` [PATCH 2/2] drm/amdgpu: Example Usage in AMDGPU Fangzhi Zuo
2021-08-28 11:54   ` kernel test robot
  -- strict thread matches above, loose matches on Subject: below --
2021-03-26  3:14 [PATCH] drm: Update MST First Link Slot Information Based on Encoding Format Fangzhi Zuo
2021-03-26 16:48 ` Lyude Paul

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57b116bf5d90459fac4ce605aeab9b25afca92e6.camel@redhat.com \
    --to=lyude@redhat.com \
    --cc=Bhawanpreet.lakha@amd.com \
    --cc=Harry.Wentland@amd.com \
    --cc=Jerry.Zuo@amd.com \
    --cc=Nicholas.Kazlauskas@amd.com \
    --cc=Wayne.Lin@amd.com \
    --cc=dri-devel@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).