From: Daniel Vetter <daniel@ffwll.ch>
To: Paulo Zanoni <przanoni@gmail.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 06/10] drm/i915: add pipe_config->has_pch_encoder
Date: Sun, 3 Mar 2013 19:13:06 +0100 [thread overview]
Message-ID: <20130303181306.GP9021@phenom.ffwll.local> (raw)
In-Reply-To: <CA+gsUGQKpHDUHVVDz0ghPCTXwd4nQ8WivpF0CsmK2y1mQ4DtOw@mail.gmail.com>
On Tue, Feb 26, 2013 at 02:39:15PM -0300, Paulo Zanoni wrote:
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 3446e2b..c5f7c6e 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -2947,27 +2947,6 @@ static void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
> > mutex_unlock(&dev->struct_mutex);
> > }
> >
> > -static bool ironlake_crtc_driving_pch(struct drm_crtc *crtc)
> > -{
> > - struct drm_device *dev = crtc->dev;
> > - struct intel_encoder *intel_encoder;
> > -
> > - /*
> > - * If there's a non-PCH eDP on this crtc, it must be DP_A, and that
> > - * must be driven by its own crtc; no sharing is possible.
> > - */
> > - for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
> > - switch (intel_encoder->type) {
> > - case INTEL_OUTPUT_EDP:
> > - if (!intel_encoder_is_pch_edp(&intel_encoder->base))
> > - return false;
> > - continue;
> > - }
> > - }
> > -
> > - return true;
> > -}
> > -
> > static bool haswell_crtc_driving_pch(struct drm_crtc *crtc)
>
> You forgot to kill haswell_crtc_driving_pch and replace its usage
> inside haswell_crtc_disable.
Nope, see the v2 comment in the commit message: Due to missing hw state
readout support we can't use that flag in the disable paths yet, hence
that code needs to stay for now. Note that I replace all usages of
haswell_crtc_driving_pch outside of the haswell crtc disable functions.
Ack for the other comments below, the IS_HASWELL/HAS_DDI is simply rebase
fail.
Thanks, Daniel
>
>
> > {
> > return intel_pipe_has_type(crtc, INTEL_OUTPUT_ANALOG);
> > @@ -3308,7 +3287,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> > int pipe = intel_crtc->pipe;
> > int plane = intel_crtc->plane;
> > u32 temp;
> > - bool is_pch_port;
> >
> > WARN_ON(!crtc->enabled);
> >
> > @@ -3324,9 +3302,8 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> > I915_WRITE(PCH_LVDS, temp | LVDS_PORT_EN);
> > }
> >
> > - is_pch_port = ironlake_crtc_driving_pch(crtc);
> >
> > - if (is_pch_port) {
> > + if (intel_crtc->config.has_pch_encoder) {
> > /* Note: FDI PLL enabling _must_ be done before we enable the
> > * cpu pipes, hence this is separate from all the other fdi/pch
> > * enabling. */
> > @@ -3363,10 +3340,11 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
> > */
> > intel_crtc_load_lut(crtc);
> >
> > - intel_enable_pipe(dev_priv, pipe, is_pch_port);
> > + intel_enable_pipe(dev_priv, pipe,
> > + intel_crtc->config.has_pch_encoder);
> > intel_enable_plane(dev_priv, plane, pipe);
> >
> > - if (is_pch_port)
> > + if (intel_crtc->config.has_pch_encoder)
> > ironlake_pch_enable(crtc);
> >
> > mutex_lock(&dev->struct_mutex);
> > @@ -3400,7 +3378,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> > struct intel_encoder *encoder;
> > int pipe = intel_crtc->pipe;
> > int plane = intel_crtc->plane;
> > - bool is_pch_port;
> >
> > WARN_ON(!crtc->enabled);
> >
> > @@ -3410,9 +3387,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> > intel_crtc->active = true;
> > intel_update_watermarks(dev);
> >
> > - is_pch_port = haswell_crtc_driving_pch(crtc);
> > -
> > - if (is_pch_port)
> > + if (intel_crtc->config.has_pch_encoder)
> > dev_priv->display.fdi_link_train(crtc);
> >
> > for_each_encoder_on_crtc(dev, crtc, encoder)
> > @@ -3443,10 +3418,11 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> > intel_ddi_set_pipe_settings(crtc);
> > intel_ddi_enable_pipe_func(crtc);
> >
> > - intel_enable_pipe(dev_priv, pipe, is_pch_port);
> > + intel_enable_pipe(dev_priv, pipe,
> > + intel_crtc->config.has_pch_encoder);
> > intel_enable_plane(dev_priv, plane, pipe);
> >
> > - if (is_pch_port)
> > + if (intel_crtc->config.has_pch_encoder)
> > lpt_pch_enable(crtc);
> >
> > mutex_lock(&dev->struct_mutex);
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 9b1a969..e851fdd 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -689,12 +689,13 @@ intel_dp_i2c_init(struct intel_dp *intel_dp,
> > }
> >
> > bool
> > -intel_dp_mode_fixup(struct drm_encoder *encoder,
> > - const struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > +intel_dp_compute_config(struct intel_encoder *encoder,
> > + struct intel_crtc_config *pipe_config)
> > {
> > - struct drm_device *dev = encoder->dev;
> > - struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > + struct drm_device *dev = encoder->base.dev;
> > + struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> > + struct drm_display_mode *mode = &pipe_config->requested_mode;
> > + struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
> > struct intel_connector *intel_connector = intel_dp->attached_connector;
> > int lane_count, clock;
> > int max_lane_count = drm_dp_max_lane_count(intel_dp->dpcd);
> > @@ -702,6 +703,9 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
> > int bpp, mode_rate;
> > static int bws[2] = { DP_LINK_BW_1_62, DP_LINK_BW_2_7 };
> >
> > + if (HAS_PCH_SPLIT(dev) && !IS_HASWELL(dev) && !is_cpu_edp(intel_dp))
>
> s/IS_HASWELL/HAS_DDI/ ?
>
>
> > + pipe_config->has_pch_encoder = true;
> > +
> > if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) {
> > intel_fixed_panel_mode(intel_connector->panel.fixed_mode,
> > adjusted_mode);
> > @@ -2517,7 +2521,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> > }
> >
> > static const struct drm_encoder_helper_funcs intel_dp_helper_funcs = {
> > - .mode_fixup = intel_dp_mode_fixup,
> > .mode_set = intel_dp_mode_set,
> > };
> >
> > @@ -2937,6 +2940,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> > DRM_MODE_ENCODER_TMDS);
> > drm_encoder_helper_add(&intel_encoder->base, &intel_dp_helper_funcs);
> >
> > + intel_encoder->compute_config = intel_dp_compute_config;
> > intel_encoder->enable = intel_enable_dp;
> > intel_encoder->pre_enable = intel_pre_enable_dp;
> > intel_encoder->disable = intel_disable_dp;
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index ef5111b..458e6bd 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -189,6 +189,9 @@ struct intel_crtc_config {
> > * changes the crtc timings in the mode to prevent the crtc fixup from
> > * overwriting them. Currently only lvds needs that. */
> > bool timings_set;
> > + /* Whether to set up the PCH/FDI. Note that we never allow sharing
> > + * between pch encoders and cpu encoders. */
> > + bool has_pch_encoder;
> > /* Used by SDVO (and if we ever fix it, HDMI). */
> > unsigned pixel_multiplier;
> > };
> > @@ -444,9 +447,8 @@ extern void intel_hdmi_init(struct drm_device *dev,
> > extern void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> > struct intel_connector *intel_connector);
> > extern struct intel_hdmi *enc_to_intel_hdmi(struct drm_encoder *encoder);
> > -extern bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
> > - const struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode);
> > +extern bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> > + struct intel_crtc_config *);
>
> To keep the coding style consistent, "struct intel_crtc_config *pipe_config);" ?
>
>
> > extern void intel_dip_infoframe_csum(struct dip_infoframe *avi_if);
> > extern bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg,
> > bool is_sdvob);
> > @@ -470,9 +472,8 @@ extern void intel_dp_complete_link_train(struct intel_dp *intel_dp);
> > extern void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode);
> > extern void intel_dp_encoder_destroy(struct drm_encoder *encoder);
> > extern void intel_dp_check_link_status(struct intel_dp *intel_dp);
> > -extern bool intel_dp_mode_fixup(struct drm_encoder *encoder,
> > - const struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode);
> > +extern bool intel_dp_compute_config(struct intel_encoder *encoder,
> > + struct intel_crtc_config *pipe_config);
> > extern bool intel_dpd_is_edp(struct drm_device *dev);
> > extern void ironlake_edp_backlight_on(struct intel_dp *intel_dp);
> > extern void ironlake_edp_backlight_off(struct intel_dp *intel_dp);
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 92aba3c..cfc9d751 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -772,11 +772,12 @@ static int intel_hdmi_mode_valid(struct drm_connector *connector,
> > return MODE_OK;
> > }
> >
> > -bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
> > - const struct drm_display_mode *mode,
> > - struct drm_display_mode *adjusted_mode)
> > +bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> > + struct intel_crtc_config *pipe_config)
> > {
> > - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> > + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
> > + struct drm_device *dev = encoder->base.dev;
> > + struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> >
> > if (intel_hdmi->color_range_auto) {
> > /* See CEA-861-E - 5.1 Default Encoding Parameters */
> > @@ -790,6 +791,9 @@ bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
> > if (intel_hdmi->color_range)
> > adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
> >
> > + if (HAS_PCH_SPLIT(dev) && !IS_HASWELL(dev))
>
> s/IS_HASWELL/HAS_DDI/ ?
>
>
> > + pipe_config->has_pch_encoder = true;
> > +
> > return true;
> > }
> >
> > @@ -941,7 +945,6 @@ static void intel_hdmi_destroy(struct drm_connector *connector)
> > }
> >
> > static const struct drm_encoder_helper_funcs intel_hdmi_helper_funcs = {
> > - .mode_fixup = intel_hdmi_mode_fixup,
> > .mode_set = intel_hdmi_mode_set,
> > };
> >
> > @@ -1069,6 +1072,7 @@ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port)
> > DRM_MODE_ENCODER_TMDS);
> > drm_encoder_helper_add(&intel_encoder->base, &intel_hdmi_helper_funcs);
> >
> > + intel_encoder->compute_config = intel_hdmi_compute_config;
> > intel_encoder->enable = intel_enable_hdmi;
> > intel_encoder->disable = intel_disable_hdmi;
> > intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
> > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> > index 1616f53..1f2efac 100644
> > --- a/drivers/gpu/drm/i915/intel_lvds.c
> > +++ b/drivers/gpu/drm/i915/intel_lvds.c
> > @@ -331,6 +331,8 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder,
> > adjusted_mode);
> >
> > if (HAS_PCH_SPLIT(dev)) {
> > + pipe_config->has_pch_encoder = true;
> > +
> > intel_pch_panel_fitting(dev,
> > intel_connector->panel.fitting_mode,
> > mode, adjusted_mode);
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 3a45be0..7100196 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -1045,6 +1045,9 @@ static bool intel_sdvo_compute_config(struct intel_encoder *encoder,
> > struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
> > struct drm_display_mode *mode = &pipe_config->requested_mode;
> >
> > + if (HAS_PCH_SPLIT(encoder->base.dev))
> > + pipe_config->has_pch_encoder = true;
> > +
> > /* We need to construct preferred input timings based on our
> > * output timings. To do that, we have to set the output
> > * timings, even though this isn't really the right place in
> > --
> > 1.7.11.4
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>
>
> --
> Paulo Zanoni
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
next prev parent reply other threads:[~2013-03-03 18:10 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-21 23:56 [PATCH 00/10] basic infrastructure for pipe_config Daniel Vetter
2013-02-21 23:56 ` [PATCH 01/10] drm/i915: introduce struct intel_crtc_config Daniel Vetter
2013-02-22 10:23 ` Ville Syrjälä
2013-02-22 11:05 ` [PATCH] " Daniel Vetter
2013-02-26 17:09 ` Paulo Zanoni
2013-02-21 23:56 ` [PATCH 02/10] drm/i915: compute pipe_config earlier Daniel Vetter
2013-02-26 17:11 ` Paulo Zanoni
2013-02-21 23:56 ` [PATCH 03/10] drm/i915: add pipe_config->timings_set Daniel Vetter
2013-02-22 13:51 ` Ville Syrjälä
2013-03-03 18:01 ` Daniel Vetter
2013-03-04 14:18 ` Ville Syrjälä
2013-02-26 17:23 ` Paulo Zanoni
2013-03-03 17:58 ` Daniel Vetter
2013-02-21 23:56 ` [PATCH 04/10] drm/i915: add pipe_config->pixel_multiplier Daniel Vetter
2013-02-26 17:27 ` Paulo Zanoni
2013-03-03 18:05 ` Daniel Vetter
2013-02-21 23:56 ` [PATCH 05/10] drm/i915: drop helper vtable for sdvo encoder Daniel Vetter
2013-02-21 23:56 ` [PATCH 06/10] drm/i915: add pipe_config->has_pch_encoder Daniel Vetter
2013-02-26 17:39 ` Paulo Zanoni
2013-03-03 18:13 ` Daniel Vetter [this message]
2013-03-03 18:16 ` [PATCH] " Daniel Vetter
2013-03-04 22:24 ` Daniel Vetter
2013-02-21 23:56 ` [PATCH 07/10] drm/i915: add pipe_config->limited_color_range Daniel Vetter
2013-02-26 17:46 ` Paulo Zanoni
2013-02-21 23:56 ` [PATCH 08/10] drm/i915: move pipe bpp computation to pipe_config Daniel Vetter
2013-03-26 21:12 ` Jesse Barnes
2013-03-26 21:32 ` Daniel Vetter
2013-02-21 23:56 ` [PATCH 09/10] drm/i915: clean up plane bpp confusion Daniel Vetter
2013-03-26 21:04 ` Jesse Barnes
2013-02-21 23:56 ` [PATCH 10/10] drm/i915: clean up pipe " Daniel Vetter
2013-03-26 21:06 ` Jesse Barnes
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=20130303181306.GP9021@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
--cc=przanoni@gmail.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.