lkml.org 
[lkml]   [2018]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v3 1/5] drm/i915: Move DP modeset retry work into intel_dp
    On Fri, Mar 09, 2018 at 04:32:27PM -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.
    >
    > Signed-off-by: Lyude Paul <lyude@redhat.com>
    > Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
    > Cc: Manasi Navare <manasi.d.navare@intel.com>
    > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
    >
    > V2:
    > - Remove accidental duplicate modeset_retry_work in intel_connector
    > V3:
    > - Also check against eDP in intel_hpd_poll_fini() - mdnavare
    > Signed-off-by: Lyude Paul <lyude@redhat.com>
    > ---
    > drivers/gpu/drm/i915/intel_display.c | 25 +++++++++++++++++++++++--
    > 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, 31 insertions(+), 12 deletions(-)
    >
    > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
    > index f424fff477f6..3b7fa430a84a 100644
    > --- a/drivers/gpu/drm/i915/intel_display.c
    > +++ b/drivers/gpu/drm/i915/intel_display.c
    > @@ -15394,16 +15394,37 @@ 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;
    > + int conn_type;
    >
    > /* 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);
    > }
    > +
    > + conn_type = connector->base.connector_type;
    > + if (conn_type != DRM_MODE_CONNECTOR_eDP &&
    > + conn_type != DRM_MODE_CONNECTOR_DisplayPort)
    > + 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);

    Why are we even walking the connectors for this? Can't we just
    walk the encoders instead?

    > }
    > 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

    --
    Ville Syrjälä
    Intel OTC

    \
     
     \ /
      Last update: 2018-03-12 22:02    [W:3.241 / U:1.004 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site