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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox