From: Manasi Navare <manasi.d.navare@intel.com>
To: Lyude Paul <lyude@redhat.com>
Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Jani Nikula" <jani.nikula@linux.intel.com>,
"Joonas Lahtinen" <joonas.lahtinen@linux.intel.com>,
"Rodrigo Vivi" <rodrigo.vivi@intel.com>,
"David Airlie" <airlied@linux.ie>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/6] drm/i915: Move DP modeset retry work into intel_dp
Date: Thu, 8 Mar 2018 23:25:35 -0800 [thread overview]
Message-ID: <20180309072535.GB2374@intel.com> (raw)
In-Reply-To: <20180308234122.16641-1-lyude@redhat.com>
On Thu, Mar 08, 2018 at 06:41:22PM -0500, Lyude Paul wrote:
> While having the modeset_retry_work in intel_connector makes sense with
> SST, this paradigm doesn't make a whole ton of sense when it comes to
> MST since we have to deal with multiple connectors. In most cases, it's
> more useful to just use the intel_dp struct since it indicates whether
> or not we're dealing with an MST device, along with being able to easily
> trace the intel_dp struct back to it's respective connector (if there is
> any). So, move the modeset_retry_work function out of the
> intel_connector struct and into intel_dp.
>
> V2:
> - Remove accidental duplicate modeset_retry_work in intel_connector
>
> Signed-off-by: Lyude Paul <lyude@redhat.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
> drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++++++++++--
> drivers/gpu/drm/i915/intel_dp.c | 10 ++++------
> drivers/gpu/drm/i915/intel_dp_link_training.c | 2 +-
> drivers/gpu/drm/i915/intel_drv.h | 6 +++---
> 4 files changed, 29 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f424fff477f6..85d5af4e4f5b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15394,16 +15394,35 @@ static void intel_hpd_poll_fini(struct drm_device *dev)
> {
> struct intel_connector *connector;
> struct drm_connector_list_iter conn_iter;
> + struct work_struct *work;
>
> /* Kill all the work that may have been queued by hpd. */
> drm_connector_list_iter_begin(dev, &conn_iter);
> for_each_intel_connector_iter(connector, &conn_iter) {
> - if (connector->modeset_retry_work.func)
> - cancel_work_sync(&connector->modeset_retry_work);
> if (connector->hdcp_shim) {
> cancel_delayed_work_sync(&connector->hdcp_check_work);
> cancel_work_sync(&connector->hdcp_prop_work);
> }
> +
> + if (connector->base.connector_type !=
> + DRM_MODE_CONNECTOR_DisplayPort)
I know we dont fallback on eDP now but since link training is a common code between
eDP and DP, this modeset retry can also be called from eDP. So we should check against
DRM_MODE_CONNECTOR_eDP type as well.
Other than that, I agree that it is better for this work function to be part of intel_dp struct
as opposed to intel_connector.
So after this change,
Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Manasi
> + continue;
> +
> + if (connector->mst_port) {
> + work = &connector->mst_port->modeset_retry_work;
> + } else {
> + struct intel_encoder *intel_encoder =
> + connector->encoder;
> + struct intel_dp *intel_dp;
> +
> + if (!intel_encoder)
> + continue;
> +
> + intel_dp = enc_to_intel_dp(&intel_encoder->base);
> + work = &intel_dp->modeset_retry_work;
> + }
> +
> + cancel_work_sync(work);
> }
> drm_connector_list_iter_end(&conn_iter);
> }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 4dd1b2287dd6..5abf0c95725a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -6261,12 +6261,10 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>
> static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
> {
> - struct intel_connector *intel_connector;
> - struct drm_connector *connector;
> + struct intel_dp *intel_dp = container_of(work, typeof(*intel_dp),
> + modeset_retry_work);
> + struct drm_connector *connector = &intel_dp->attached_connector->base;
>
> - intel_connector = container_of(work, typeof(*intel_connector),
> - modeset_retry_work);
> - connector = &intel_connector->base;
> DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
> connector->name);
>
> @@ -6295,7 +6293,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
> int type;
>
> /* Initialize the work for modeset in case of link train failure */
> - INIT_WORK(&intel_connector->modeset_retry_work,
> + INIT_WORK(&intel_dp->modeset_retry_work,
> intel_dp_modeset_retry_work_fn);
>
> if (WARN(intel_dig_port->max_lanes < 1,
> diff --git a/drivers/gpu/drm/i915/intel_dp_link_training.c b/drivers/gpu/drm/i915/intel_dp_link_training.c
> index f59b59bb0a21..2cfa58ce1f95 100644
> --- a/drivers/gpu/drm/i915/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/intel_dp_link_training.c
> @@ -340,7 +340,7 @@ intel_dp_start_link_train(struct intel_dp *intel_dp)
> intel_dp->link_rate,
> intel_dp->lane_count))
> /* Schedule a Hotplug Uevent to userspace to start modeset */
> - schedule_work(&intel_connector->modeset_retry_work);
> + schedule_work(&intel_dp->modeset_retry_work);
> } else {
> DRM_ERROR("[CONNECTOR:%d:%s] Link Training failed at link rate = %d, lane count = %d",
> intel_connector->base.base.id,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 83e5ca889d9c..3f19dc80997f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -406,9 +406,6 @@ struct intel_connector {
>
> struct intel_dp *mst_port;
>
> - /* Work struct to schedule a uevent on link train failure */
> - struct work_struct modeset_retry_work;
> -
> const struct intel_hdcp_shim *hdcp_shim;
> struct mutex hdcp_mutex;
> uint64_t hdcp_value; /* protected by hdcp_mutex */
> @@ -1135,6 +1132,9 @@ struct intel_dp {
>
> /* Displayport compliance testing */
> struct intel_dp_compliance compliance;
> +
> + /* Work struct to schedule a uevent on link train failure */
> + struct work_struct modeset_retry_work;
> };
>
> struct intel_lspcon {
> --
> 2.14.3
>
next prev parent reply other threads:[~2018-03-09 7:29 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-08 23:24 [PATCH 0/6] Implement proper MST fallback retraining in i915 Lyude Paul
2018-03-08 23:24 ` [PATCH 1/6] drm/i915: Remove unused DP_LINK_CHECK_TIMEOUT Lyude Paul
2018-03-09 6:59 ` Manasi Navare
2018-03-08 23:24 ` [PATCH 2/6] drm/i915: Move DP modeset retry work into intel_dp Lyude Paul
2018-03-08 23:41 ` [PATCH v2 " Lyude Paul
2018-03-09 7:25 ` Manasi Navare [this message]
2018-03-08 23:24 ` [PATCH 3/6] drm/i915: Only use one link bw config for MST topologies Lyude Paul
2018-03-08 23:24 ` [PATCH 4/6] drm/dp_mst: Add drm_dp_mst_topology_mgr_lower_link_rate() Lyude Paul
2018-03-08 23:24 ` [PATCH 5/6] drm/dp_mst: Add drm_atomic_dp_mst_retrain_topology() Lyude Paul
2018-03-08 23:24 ` [PATCH 6/6] drm/i915: Implement proper fallback training for MST Lyude Paul
2018-03-09 21:32 ` [PATCH v3 1/5] drm/i915: Move DP modeset retry work into intel_dp Lyude Paul
2018-03-09 21:32 ` [PATCH v3 2/5] drm/i915: Only use one link bw config for MST topologies Lyude Paul
2018-03-12 20:45 ` Manasi Navare
2018-03-13 23:18 ` Lyude Paul
2018-03-12 21:05 ` Ville Syrjälä
2018-03-09 21:32 ` [PATCH v3 3/5] drm/dp_mst: Add drm_dp_mst_topology_mgr_lower_link_rate() Lyude Paul
2018-03-12 21:28 ` Manasi Navare
2018-03-09 21:32 ` [PATCH v3 4/5] drm/dp_mst: Add drm_atomic_dp_mst_retrain_topology() Lyude Paul
2018-03-09 21:32 ` [PATCH v3 5/5] drm/i915: Implement proper fallback training for MST Lyude Paul
2018-03-12 21:12 ` Ville Syrjälä
2018-03-12 22:05 ` Manasi Navare
2018-03-12 22:16 ` Lyude Paul
2018-03-12 21:01 ` [PATCH v3 1/5] drm/i915: Move DP modeset retry work into intel_dp Ville Syrjälä
2018-03-13 23:24 ` Lyude Paul
2018-03-14 17:28 ` Ville Syrjälä
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=20180309072535.GB2374@intel.com \
--to=manasi.d.navare@intel.com \
--cc=airlied@linux.ie \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=joonas.lahtinen@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lyude@redhat.com \
--cc=rodrigo.vivi@intel.com \
--cc=ville.syrjala@linux.intel.com \
/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).