[v8,11/17] drm/dp_mst: Add DSC enablement helpers to DRM
diff mbox series

Message ID 20191203143530.27262-12-mikita.lipski@amd.com
State New
Headers show
Series
  • DSC MST support for DRM and AMDGPU
Related show

Commit Message

Lipski, Mikita Dec. 3, 2019, 2:35 p.m. UTC
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(+)

Comments

kbuild test robot Dec. 3, 2019, 9:52 p.m. UTC | #1
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
Lyude Paul Dec. 7, 2019, 12:24 a.m. UTC | #2
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,
Mikita Lipski Dec. 9, 2019, 2:07 p.m. UTC | #3
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,

Patch
diff mbox series

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,