| Message ID | 20191203143530.27262-12-mikita.lipski@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSC MST support for DRM and AMDGPU | expand |
Hi, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on next-20191203] [cannot apply to drm-intel/for-linux-next linus/master v5.4-rc8 v5.4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/mikita-lipski-amd-com/DSC-MST-support-for-DRM-and-AMDGPU/20191204-020604 base: 1ab75b2e415a29dba9aec94f203c6f88dbfc0ba0 reproduce: make htmldocs If you fix the issue, kindly add following tag Reported-by: kbuild test robot <lkp@intel.com> All warnings (new ones prefixed by >>): drivers/usb/typec/class.c:1: warning: 'typec_altmode_register_notifier' not found fs/posix_acl.c:647: warning: Function parameter or member 'inode' not described in 'posix_acl_update_mode' fs/posix_acl.c:647: warning: Function parameter or member 'mode_p' not described in 'posix_acl_update_mode' fs/posix_acl.c:647: warning: Function parameter or member 'acl' not described in 'posix_acl_update_mode' include/linux/regulator/machine.h:196: warning: Function parameter or member 'max_uV_step' not described in 'regulation_constraints' include/linux/regulator/driver.h:223: warning: Function parameter or member 'resume' not described in 'regulator_ops' sound/soc/soc-core.c:2509: warning: Function parameter or member 'legacy_dai_naming' not described in 'snd_soc_register_dai' include/linux/skbuff.h:888: warning: Function parameter or member 'dev_scratch' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'list' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'ip_defrag_offset' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'skb_mstamp_ns' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member '__cloned_offset' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'head_frag' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member '__pkt_type_offset' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'encapsulation' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'encap_hdr_csum' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'csum_valid' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member '__pkt_vlan_present_offset' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'vlan_present' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'csum_complete_sw' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'csum_level' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'inner_protocol_type' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'remcsum_offload' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'sender_cpu' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'reserved_tailroom' not described in 'sk_buff' include/linux/skbuff.h:888: warning: Function parameter or member 'inner_ipproto' not described in 'sk_buff' include/net/sock.h:232: warning: Function parameter or member 'skc_addrpair' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_portpair' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_ipv6only' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_net_refcnt' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_v6_daddr' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_v6_rcv_saddr' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_cookie' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_listener' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_tw_dr' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_rcv_wnd' not described in 'sock_common' include/net/sock.h:232: warning: Function parameter or member 'skc_tw_rcv_nxt' not described in 'sock_common' include/net/sock.h:514: warning: Function parameter or member 'sk_rx_skb_cache' not described in 'sock' include/net/sock.h:514: warning: Function parameter or member 'sk_wq_raw' not described in 'sock' include/net/sock.h:514: warning: Function parameter or member 'tcp_rtx_queue' not described in 'sock' include/net/sock.h:514: warning: Function parameter or member 'sk_tx_skb_cache' not described in 'sock' include/net/sock.h:514: warning: Function parameter or member 'sk_route_forced_caps' not described in 'sock' include/net/sock.h:514: warning: Function parameter or member 'sk_txtime_report_errors' not described in 'sock' include/net/sock.h:514: warning: Function parameter or member 'sk_validate_xmit_skb' not described in 'sock' include/net/sock.h:514: warning: Function parameter or member 'sk_bpf_storage' not described in 'sock' include/net/sock.h:2454: warning: Function parameter or member 'tcp_rx_skb_cache_key' not described in 'DECLARE_STATIC_KEY_FALSE' include/net/sock.h:2454: warning: Excess function parameter 'sk' description in 'DECLARE_STATIC_KEY_FALSE' include/net/sock.h:2454: warning: Excess function parameter 'skb' description in 'DECLARE_STATIC_KEY_FALSE' include/linux/netdevice.h:1779: warning: bad line: spinlock include/linux/netdevice.h:2077: warning: Function parameter or member 'gso_partial_features' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'l3mdev_ops' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'xfrmdev_ops' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'tlsdev_ops' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'name_assign_type' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'ieee802154_ptr' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'mpls_ptr' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'xdp_prog' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'gro_flush_timeout' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'nf_hooks_ingress' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'qdisc_hash' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'xps_cpus_map' not described in 'net_device' include/linux/netdevice.h:2077: warning: Function parameter or member 'xps_rxqs_map' not described in 'net_device' include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(advertising' not described in 'phylink_link_state' include/linux/phylink.h:56: warning: Function parameter or member '__ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising' not described in 'phylink_link_state' include/linux/i2c.h:337: warning: Function parameter or member 'init_irq' not described in 'i2c_client' drivers/infiniband/core/umem_odp.c:167: warning: Function parameter or member 'ops' not described in 'ib_umem_odp_alloc_child' drivers/infiniband/core/umem_odp.c:217: warning: Function parameter or member 'ops' not described in 'ib_umem_odp_get' drivers/infiniband/ulp/iser/iscsi_iser.h:401: warning: Function parameter or member 'all_list' not described in 'iser_fr_desc' drivers/infiniband/ulp/iser/iscsi_iser.h:415: warning: Function parameter or member 'all_list' not described in 'iser_fr_pool' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd0' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd1' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd2' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd3' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:148: warning: Function parameter or member 'rsvd4' not described in 'opa_vesw_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd0' not described in 'opa_per_veswport_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd1' not described in 'opa_per_veswport_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd2' not described in 'opa_per_veswport_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:205: warning: Function parameter or member 'rsvd3' not described in 'opa_per_veswport_info' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:263: warning: Function parameter or member 'tbl_entries' not described in 'opa_veswport_mactable' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:342: warning: Function parameter or member 'reserved' not described in 'opa_veswport_summary_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd0' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd1' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd2' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd3' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd4' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd5' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd6' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd7' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd8' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:394: warning: Function parameter or member 'rsvd9' not described in 'opa_veswport_error_counters' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:460: warning: Function parameter or member 'reserved' not described in 'opa_vnic_vema_mad' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:485: warning: Function parameter or member 'reserved' not described in 'opa_vnic_notice_attr' drivers/infiniband/ulp/opa_vnic/opa_vnic_encap.h:500: warning: Function parameter or member 'reserved' not described in 'opa_vnic_vema_mad_trap' include/linux/input/sparse-keymap.h:43: warning: Function parameter or member 'sw' not described in 'key_entry' kernel/futex.c:1187: warning: Function parameter or member 'ret' not described in 'wait_for_owner_exiting' include/drm/drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'prepare_writeback_job' not described in 'drm_connector_helper_funcs' include/drm/drm_modeset_helper_vtables.h:1052: warning: Function parameter or member 'cleanup_writeback_job' not described in 'drm_connector_helper_funcs' drivers/gpu/drm/drm_dp_mst_topology.c:4763: warning: Excess function parameter 'pointer' description in 'drm_dp_mst_atomic_enable_dsc' drivers/gpu/drm/drm_dp_mst_topology.c:4763: warning: Excess function parameter 'pointer' description in 'drm_dp_mst_atomic_enable_dsc' include/drm/drm_dp_mst_helper.h:162: warning: Function parameter or member 'fec_capable' not described in 'drm_dp_mst_port' >> drivers/gpu/drm/drm_dp_mst_topology.c:4764: warning: Function parameter or member 'port' not described in 'drm_dp_mst_atomic_enable_dsc' drivers/gpu/drm/drm_dp_mst_topology.c:4764: warning: Excess function parameter 'pointer' description in 'drm_dp_mst_atomic_enable_dsc' drivers/gpu/drm/drm_dp_mst_topology.c:4763: warning: Excess function parameter 'pointer' description in 'drm_dp_mst_atomic_enable_dsc' include/net/cfg80211.h:1189: warning: Function parameter or member 'txpwr' not described in 'station_parameters' include/net/mac80211.h:4081: warning: Function parameter or member 'sta_set_txpwr' not described in 'ieee80211_ops' include/net/mac80211.h:2036: warning: Function parameter or member 'txpwr' not described in 'ieee80211_sta' include/linux/devfreq.h:181: warning: Function parameter or member 'last_status' not described in 'devfreq' drivers/devfreq/devfreq.c:1708: warning: bad line: - Resource-managed devfreq_register_notifier() drivers/devfreq/devfreq.c:1744: warning: bad line: - Resource-managed devfreq_unregister_notifier() drivers/devfreq/devfreq-event.c:355: warning: Function parameter or member 'edev' not described in 'devfreq_event_remove_edev' drivers/devfreq/devfreq-event.c:355: warning: Excess function parameter 'dev' description in 'devfreq_event_remove_edev' Documentation/admin-guide/xfs.rst:257: WARNING: Block quote ends without a blank line; unexpected unindent. Documentation/admin-guide/hw-vuln/tsx_async_abort.rst:142: WARNING: duplicate label virt_mechanism, other instance in Documentation/admin-guide/hw-vuln/mds.rst Documentation/admin-guide/ras.rst:358: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/admin-guide/ras.rst:358: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/admin-guide/ras.rst:363: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/admin-guide/ras.rst:363: WARNING: Definition list ends without a blank line; unexpected unindent. Documentation/admin-guide/sysctl/kernel.rst:397: WARNING: Title underline too short. vim +4764 drivers/gpu/drm/drm_dp_mst_topology.c 4744 4745 /** 4746 * drm_dp_mst_atomic_enable_dsc - Set DSC Enable Flag to On/Off 4747 * @state: Pointer to the new drm_atomic_state 4748 * @pointer: Pointer to the affected MST Port 4749 * @pbn: Newly recalculated bw required for link with DSC enabled 4750 * @pbn_div: Divider to calculate correct number of pbn per slot 4751 * @enable: Boolean flag enabling or disabling DSC on the port 4752 * 4753 * This function enables DSC on the given Port 4754 * by recalculating its vcpi from pbn provided 4755 * and sets dsc_enable flag to keep track of which 4756 * ports have DSC enabled 4757 * 4758 */ 4759 int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state, 4760 struct drm_dp_mst_port *port, 4761 int pbn, int pbn_div, 4762 bool enable) > 4763 { > 4764 struct drm_dp_mst_topology_state *mst_state; 4765 struct drm_dp_vcpi_allocation *pos; 4766 bool found = false; 4767 int vcpi = 0; 4768 4769 mst_state = drm_atomic_get_mst_topology_state(state, port->mgr); 4770 4771 if (IS_ERR(mst_state)) 4772 return PTR_ERR(mst_state); 4773 4774 list_for_each_entry(pos, &mst_state->vcpis, next) { 4775 if (pos->port == port) { 4776 found = true; 4777 break; 4778 } 4779 } 4780 4781 if (!found) { 4782 DRM_DEBUG_ATOMIC("[MST PORT:%p] Couldn't find VCPI allocation in mst state %p\n", 4783 port, mst_state); 4784 return -EINVAL; 4785 } 4786 4787 if (pos->dsc_enabled == enable) { 4788 DRM_DEBUG_ATOMIC("[MST PORT:%p] DSC flag is already set to %d, returning %d VCPI slots\n", 4789 port, enable, pos->vcpi); 4790 vcpi = pos->vcpi; 4791 } 4792 4793 if (enable) { 4794 vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr, port, pbn, pbn_div); 4795 DRM_DEBUG_ATOMIC("[MST PORT:%p] Enabling DSC flag, reallocating %d VCPI slots on the port\n", 4796 port, vcpi); 4797 if (vcpi < 0) 4798 return -EINVAL; 4799 } 4800 4801 pos->dsc_enabled = enable; 4802 4803 return vcpi; 4804 } 4805 EXPORT_SYMBOL(drm_dp_mst_atomic_enable_dsc); 4806 /** 4807 * drm_dp_mst_atomic_check - Check that the new state of an MST topology in an 4808 * atomic update is valid 4809 * @state: Pointer to the new &struct drm_dp_mst_topology_state 4810 * 4811 * Checks the given topology state for an atomic update to ensure that it's 4812 * valid. This includes checking whether there's enough bandwidth to support 4813 * the new VCPI allocations in the atomic update. 4814 * 4815 * Any atomic drivers supporting DP MST must make sure to call this after 4816 * checking the rest of their state in their 4817 * &drm_mode_config_funcs.atomic_check() callback. 4818 * 4819 * See also: 4820 * drm_dp_atomic_find_vcpi_slots() 4821 * drm_dp_atomic_release_vcpi_slots() 4822 * 4823 * Returns: 4824 * 4825 * 0 if the new state is valid, negative error code otherwise. 4826 */ 4827 int drm_dp_mst_atomic_check(struct drm_atomic_state *state) 4828 { 4829 struct drm_dp_mst_topology_mgr *mgr; 4830 struct drm_dp_mst_topology_state *mst_state; 4831 int i, ret = 0; 4832 4833 for_each_new_mst_mgr_in_state(state, mgr, mst_state, i) { 4834 ret = drm_dp_mst_atomic_check_topology_state(mgr, mst_state); 4835 if (ret) 4836 break; 4837 } 4838 4839 return ret; 4840 } 4841 EXPORT_SYMBOL(drm_dp_mst_atomic_check); 4842 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation
Nice! All I've got is a couple of typos I noticed and one question, this looks great :) On Tue, 2019-12-03 at 09:35 -0500, mikita.lipski@amd.com wrote: > From: Mikita Lipski <mikita.lipski@amd.com> > > Adding a helper function to be called by > drivers outside of DRM to enable DSC on > the MST ports. > > Function is called to recalculate VCPI allocation > if DSC is enabled and raise the DSC flag to enable. > In case of disabling DSC the flag is set to false > and recalculation of VCPI slots is expected to be done > in encoder's atomic_check. > > v2: squash separate functions into one and call it per > port > > Cc: Harry Wentland <harry.wentland@amd.com> > Cc: Lyude Paul <lyude@redhat.com> > Signed-off-by: Mikita Lipski <mikita.lipski@amd.com> > --- > drivers/gpu/drm/drm_dp_mst_topology.c | 61 +++++++++++++++++++++++++++ > include/drm/drm_dp_mst_helper.h | 5 +++ > 2 files changed, 66 insertions(+) > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index f1d883960831..5e549f48ffb8 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -4742,6 +4742,67 @@ drm_dp_mst_atomic_check_topology_state(struct > drm_dp_mst_topology_mgr *mgr, > return 0; > } > > +/** > + * drm_dp_mst_atomic_enable_dsc - Set DSC Enable Flag to On/Off > + * @state: Pointer to the new drm_atomic_state > + * @pointer: Pointer to the affected MST Port Typo here > + * @pbn: Newly recalculated bw required for link with DSC enabled > + * @pbn_div: Divider to calculate correct number of pbn per slot > + * @enable: Boolean flag enabling or disabling DSC on the port > + * > + * This function enables DSC on the given Port > + * by recalculating its vcpi from pbn provided > + * and sets dsc_enable flag to keep track of which > + * ports have DSC enabled > + * > + */ > +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state, > + struct drm_dp_mst_port *port, > + int pbn, int pbn_div, > + bool enable) > +{ > + struct drm_dp_mst_topology_state *mst_state; > + struct drm_dp_vcpi_allocation *pos; > + bool found = false; > + int vcpi = 0; > + > + mst_state = drm_atomic_get_mst_topology_state(state, port->mgr); > + > + if (IS_ERR(mst_state)) > + return PTR_ERR(mst_state); > + > + list_for_each_entry(pos, &mst_state->vcpis, next) { > + if (pos->port == port) { > + found = true; > + break; > + } > + } > + > + if (!found) { > + DRM_DEBUG_ATOMIC("[MST PORT:%p] Couldn't find VCPI allocation > in mst state %p\n", > + port, mst_state); > + return -EINVAL; > + } Just double checking-does this handle the case where we're enabling DSC on a port that didn't previously have a VCPI allocation because it wasn't enabled previously? Or do we not need to handle that here Assuming you did the right thing here, with the small typo fixes: Reviewed-by: Lyude Paul <lyude@redhat.com> > + > + if (pos->dsc_enabled == enable) { > + DRM_DEBUG_ATOMIC("[MST PORT:%p] DSC flag is already set to %d, > returning %d VCPI slots\n", > + port, enable, pos->vcpi); > + vcpi = pos->vcpi; > + } > + > + if (enable) { > + vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr, port, > pbn, pbn_div); > + DRM_DEBUG_ATOMIC("[MST PORT:%p] Enabling DSC flag, > reallocating %d VCPI slots on the port\n", > + port, vcpi); > + if (vcpi < 0) > + return -EINVAL; > + } > + > + pos->dsc_enabled = enable; > + > + return vcpi; > +} > +EXPORT_SYMBOL(drm_dp_mst_atomic_enable_dsc); > /** > * drm_dp_mst_atomic_check - Check that the new state of an MST topology in > an > * atomic update is valid > diff --git a/include/drm/drm_dp_mst_helper.h > b/include/drm/drm_dp_mst_helper.h > index 0f813d6346aa..830c94b7f45d 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -502,6 +502,7 @@ struct drm_dp_payload { > struct drm_dp_vcpi_allocation { > struct drm_dp_mst_port *port; > int vcpi; > + bool dsc_enabled; > struct list_head next; > }; > > @@ -773,6 +774,10 @@ drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state > *state, > struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port, int pbn, > int pbn_div); > +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state, > + struct drm_dp_mst_port *port, > + int pbn, int pbn_div, > + bool enable); > int __must_check > drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, > struct drm_dp_mst_topology_mgr *mgr,
On 12/6/19 7:24 PM, Lyude Paul wrote: > Nice! All I've got is a couple of typos I noticed and one question, this looks > great :) > Thanks! I'll clean it up. The response to the question is below. >> On Tue, 2019-12-03 at 09:35 -0500, mikita.lipski@amd.com wrote: >> From: Mikita Lipski <mikita.lipski@amd.com> >> >> Adding a helper function to be called by >> drivers outside of DRM to enable DSC on >> the MST ports. >> >> Function is called to recalculate VCPI allocation >> if DSC is enabled and raise the DSC flag to enable. >> In case of disabling DSC the flag is set to false >> and recalculation of VCPI slots is expected to be done >> in encoder's atomic_check. >> >> v2: squash separate functions into one and call it per >> port >> >> Cc: Harry Wentland <harry.wentland@amd.com> >> Cc: Lyude Paul <lyude@redhat.com> >> Signed-off-by: Mikita Lipski <mikita.lipski@amd.com> >> --- >> drivers/gpu/drm/drm_dp_mst_topology.c | 61 +++++++++++++++++++++++++++ >> include/drm/drm_dp_mst_helper.h | 5 +++ >> 2 files changed, 66 insertions(+) >> >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c >> b/drivers/gpu/drm/drm_dp_mst_topology.c >> index f1d883960831..5e549f48ffb8 100644 >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c >> @@ -4742,6 +4742,67 @@ drm_dp_mst_atomic_check_topology_state(struct >> drm_dp_mst_topology_mgr *mgr, >> return 0; >> } >> >> +/** >> + * drm_dp_mst_atomic_enable_dsc - Set DSC Enable Flag to On/Off >> + * @state: Pointer to the new drm_atomic_state >> + * @pointer: Pointer to the affected MST Port > Typo here > >> + * @pbn: Newly recalculated bw required for link with DSC enabled >> + * @pbn_div: Divider to calculate correct number of pbn per slot >> + * @enable: Boolean flag enabling or disabling DSC on the port >> + * >> + * This function enables DSC on the given Port >> + * by recalculating its vcpi from pbn provided >> + * and sets dsc_enable flag to keep track of which >> + * ports have DSC enabled >> + * >> + */ >> +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state, >> + struct drm_dp_mst_port *port, >> + int pbn, int pbn_div, >> + bool enable) >> +{ >> + struct drm_dp_mst_topology_state *mst_state; >> + struct drm_dp_vcpi_allocation *pos; >> + bool found = false; >> + int vcpi = 0; >> + >> + mst_state = drm_atomic_get_mst_topology_state(state, port->mgr); >> + >> + if (IS_ERR(mst_state)) >> + return PTR_ERR(mst_state); >> + >> + list_for_each_entry(pos, &mst_state->vcpis, next) { >> + if (pos->port == port) { >> + found = true; >> + break; >> + } >> + } >> + >> + if (!found) { >> + DRM_DEBUG_ATOMIC("[MST PORT:%p] Couldn't find VCPI allocation >> in mst state %p\n", >> + port, mst_state); >> + return -EINVAL; >> + } > > Just double checking-does this handle the case where we're enabling DSC on a > port that didn't previously have a VCPI allocation because it wasn't enabled > previously? Or do we not need to handle that here Because we call encoder atomic check previously to allocate VCPI slots - the port should have VCPI allocation before enabling DSC even if it wasn't enabled previously. Therefore, I was thinking, that if encoder atomic check fails to allocate VCPI slots for the port - we shouldn't enable DSC on it and probably should fail atomic check if that is even requested. > > Assuming you did the right thing here, with the small typo fixes: > > Reviewed-by: Lyude Paul <lyude@redhat.com> > >> + >> + if (pos->dsc_enabled == enable) { >> + DRM_DEBUG_ATOMIC("[MST PORT:%p] DSC flag is already set to %d, >> returning %d VCPI slots\n", >> + port, enable, pos->vcpi); >> + vcpi = pos->vcpi; >> + } >> + >> + if (enable) { >> + vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr, port, >> pbn, pbn_div); >> + DRM_DEBUG_ATOMIC("[MST PORT:%p] Enabling DSC flag, >> reallocating %d VCPI slots on the port\n", >> + port, vcpi); >> + if (vcpi < 0) >> + return -EINVAL; >> + } >> + >> + pos->dsc_enabled = enable; >> + >> + return vcpi; >> +} >> +EXPORT_SYMBOL(drm_dp_mst_atomic_enable_dsc); >> /** >> * drm_dp_mst_atomic_check - Check that the new state of an MST topology in >> an >> * atomic update is valid >> diff --git a/include/drm/drm_dp_mst_helper.h >> b/include/drm/drm_dp_mst_helper.h >> index 0f813d6346aa..830c94b7f45d 100644 >> --- a/include/drm/drm_dp_mst_helper.h >> +++ b/include/drm/drm_dp_mst_helper.h >> @@ -502,6 +502,7 @@ struct drm_dp_payload { >> struct drm_dp_vcpi_allocation { >> struct drm_dp_mst_port *port; >> int vcpi; >> + bool dsc_enabled; >> struct list_head next; >> }; >> >> @@ -773,6 +774,10 @@ drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state >> *state, >> struct drm_dp_mst_topology_mgr *mgr, >> struct drm_dp_mst_port *port, int pbn, >> int pbn_div); >> +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state, >> + struct drm_dp_mst_port *port, >> + int pbn, int pbn_div, >> + bool enable); >> int __must_check >> drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, >> struct drm_dp_mst_topology_mgr *mgr,
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index f1d883960831..5e549f48ffb8 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -4742,6 +4742,67 @@ drm_dp_mst_atomic_check_topology_state(struct drm_dp_mst_topology_mgr *mgr, return 0; } +/** + * drm_dp_mst_atomic_enable_dsc - Set DSC Enable Flag to On/Off + * @state: Pointer to the new drm_atomic_state + * @pointer: Pointer to the affected MST Port + * @pbn: Newly recalculated bw required for link with DSC enabled + * @pbn_div: Divider to calculate correct number of pbn per slot + * @enable: Boolean flag enabling or disabling DSC on the port + * + * This function enables DSC on the given Port + * by recalculating its vcpi from pbn provided + * and sets dsc_enable flag to keep track of which + * ports have DSC enabled + * + */ +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state, + struct drm_dp_mst_port *port, + int pbn, int pbn_div, + bool enable) +{ + struct drm_dp_mst_topology_state *mst_state; + struct drm_dp_vcpi_allocation *pos; + bool found = false; + int vcpi = 0; + + mst_state = drm_atomic_get_mst_topology_state(state, port->mgr); + + if (IS_ERR(mst_state)) + return PTR_ERR(mst_state); + + list_for_each_entry(pos, &mst_state->vcpis, next) { + if (pos->port == port) { + found = true; + break; + } + } + + if (!found) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] Couldn't find VCPI allocation in mst state %p\n", + port, mst_state); + return -EINVAL; + } + + if (pos->dsc_enabled == enable) { + DRM_DEBUG_ATOMIC("[MST PORT:%p] DSC flag is already set to %d, returning %d VCPI slots\n", + port, enable, pos->vcpi); + vcpi = pos->vcpi; + } + + if (enable) { + vcpi = drm_dp_atomic_find_vcpi_slots(state, port->mgr, port, pbn, pbn_div); + DRM_DEBUG_ATOMIC("[MST PORT:%p] Enabling DSC flag, reallocating %d VCPI slots on the port\n", + port, vcpi); + if (vcpi < 0) + return -EINVAL; + } + + pos->dsc_enabled = enable; + + return vcpi; +} +EXPORT_SYMBOL(drm_dp_mst_atomic_enable_dsc); /** * drm_dp_mst_atomic_check - Check that the new state of an MST topology in an * atomic update is valid diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 0f813d6346aa..830c94b7f45d 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -502,6 +502,7 @@ struct drm_dp_payload { struct drm_dp_vcpi_allocation { struct drm_dp_mst_port *port; int vcpi; + bool dsc_enabled; struct list_head next; }; @@ -773,6 +774,10 @@ drm_dp_atomic_find_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int pbn, int pbn_div); +int drm_dp_mst_atomic_enable_dsc(struct drm_atomic_state *state, + struct drm_dp_mst_port *port, + int pbn, int pbn_div, + bool enable); int __must_check drm_dp_atomic_release_vcpi_slots(struct drm_atomic_state *state, struct drm_dp_mst_topology_mgr *mgr,