From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 01/10] drm/i915: introduce struct intel_crtc_config
Date: Fri, 22 Feb 2013 12:23:36 +0200 [thread overview]
Message-ID: <20130222102336.GK4469@intel.com> (raw)
In-Reply-To: <1361491014-13888-2-git-send-email-daniel.vetter@ffwll.ch>
On Fri, Feb 22, 2013 at 12:56:45AM +0100, Daniel Vetter wrote:
> Currently only containing the requested and the adjusted mode. And
> only crtc callbacks are converted somewhat to it, encoders will be
> done on a as-needed basis (simply too much churn in one patch
> otherwise).
>
> Future patches will add tons more useful stuff to this struct,
> starting with the very simple.
>
> v2: Store the pipe_config in the intel_crtc, so that the ->mode-set,
> ->enable and also ->disable have easy access to it.
>
> v3: Store the pipe config in the right crtc ...
>
> v4: Rebased.
>
> v5: Fixup an OOPS when trying to kfree an ERR_PTR.
>
> v6: Used drm_moode_copy and some other small cleanups as suggested
> by Ville Syrjälä.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 4 +-
> drivers/gpu/drm/i915/intel_display.c | 83 +++++++++++++++++++++---------------
> drivers/gpu/drm/i915/intel_drv.h | 7 +++
> 3 files changed, 58 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e95337c..8fb14fb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -271,6 +271,8 @@ struct drm_i915_error_state {
> struct intel_display_error_state *display;
> };
>
> +struct intel_crtc_config;
> +
> struct drm_i915_display_funcs {
> bool (*fbc_enabled)(struct drm_device *dev);
> void (*enable_fbc)(struct drm_crtc *crtc, unsigned long interval);
> @@ -284,8 +286,6 @@ struct drm_i915_display_funcs {
> struct drm_display_mode *mode);
> void (*modeset_global_resources)(struct drm_device *dev);
> int (*crtc_mode_set)(struct drm_crtc *crtc,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode,
> int x, int y,
> struct drm_framebuffer *old_fb);
> void (*crtc_enable)(struct drm_crtc *crtc);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b521198..5a3e231 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3915,15 +3915,15 @@ bool intel_connector_get_hw_state(struct intel_connector *connector)
> return encoder->get_hw_state(encoder, &pipe);
> }
>
> -static bool intel_crtc_mode_fixup(struct drm_crtc *crtc,
> - const struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode)
> +static bool intel_crtc_compute_config(struct drm_crtc *crtc,
> + struct intel_crtc_config *pipe_config)
> {
> struct drm_device *dev = crtc->dev;
> + struct drm_display_mode *adjusted_mode = &pipe_config->adjusted_mode;
>
> if (HAS_PCH_SPLIT(dev)) {
> /* FDI link clock is fixed at 2.7G */
> - if (mode->clock * 3 > IRONLAKE_FDI_FREQ * 4)
> + if (pipe_config->requested_mode.clock * 3 > IRONLAKE_FDI_FREQ * 4)
> return false;
> }
>
> @@ -4609,14 +4609,15 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc,
> }
>
> static int i9xx_crtc_mode_set(struct drm_crtc *crtc,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode,
> int x, int y,
> struct drm_framebuffer *fb)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_display_mode *adjusted_mode =
> + &intel_crtc->config.adjusted_mode;
> + struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
> int refclk, num_connectors = 0;
> @@ -5579,14 +5580,15 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
> }
>
> static int ironlake_crtc_mode_set(struct drm_crtc *crtc,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode,
> int x, int y,
> struct drm_framebuffer *fb)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_display_mode *adjusted_mode =
> + &intel_crtc->config.adjusted_mode;
> + struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
> int num_connectors = 0;
> @@ -5745,14 +5747,15 @@ static void haswell_modeset_global_resources(struct drm_device *dev)
> }
>
> static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode,
> int x, int y,
> struct drm_framebuffer *fb)
> {
> struct drm_device *dev = crtc->dev;
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_display_mode *adjusted_mode =
> + &intel_crtc->config.adjusted_mode;
> + struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> int pipe = intel_crtc->pipe;
> int plane = intel_crtc->plane;
> int num_connectors = 0;
> @@ -5829,8 +5832,6 @@ static int haswell_crtc_mode_set(struct drm_crtc *crtc,
> }
>
> static int intel_crtc_mode_set(struct drm_crtc *crtc,
> - struct drm_display_mode *mode,
> - struct drm_display_mode *adjusted_mode,
> int x, int y,
> struct drm_framebuffer *fb)
> {
> @@ -5839,6 +5840,9 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
> struct drm_encoder_helper_funcs *encoder_funcs;
> struct intel_encoder *encoder;
> struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> + struct drm_display_mode *adjusted_mode =
> + &intel_crtc->config.adjusted_mode;
> + struct drm_display_mode *mode = &intel_crtc->config.requested_mode;
> int pipe = intel_crtc->pipe;
> int ret;
>
> @@ -5849,8 +5853,8 @@ static int intel_crtc_mode_set(struct drm_crtc *crtc,
>
> drm_vblank_pre_modeset(dev, pipe);
>
> - ret = dev_priv->display.crtc_mode_set(crtc, mode, adjusted_mode,
> - x, y, fb);
> + ret = dev_priv->display.crtc_mode_set(crtc, x, y, fb);
> +
> drm_vblank_post_modeset(dev, pipe);
>
> if (ret != 0)
> @@ -7478,19 +7482,24 @@ static void intel_modeset_commit_output_state(struct drm_device *dev)
> }
> }
>
> -static struct drm_display_mode *
> -intel_modeset_adjusted_mode(struct drm_crtc *crtc,
> - struct drm_display_mode *mode)
> +static struct intel_crtc_config *
> +intel_modeset_pipe_config(struct drm_crtc *crtc,
> + struct drm_display_mode *mode)
> {
> struct drm_device *dev = crtc->dev;
> - struct drm_display_mode *adjusted_mode;
> struct drm_encoder_helper_funcs *encoder_funcs;
> struct intel_encoder *encoder;
> + struct intel_crtc_config *pipe_config;
>
> - adjusted_mode = drm_mode_duplicate(dev, mode);
> - if (!adjusted_mode)
> + pipe_config = kzalloc(sizeof(*pipe_config), GFP_KERNEL);
> + if (!pipe_config)
> return ERR_PTR(-ENOMEM);
>
> + drm_mode_copy(&pipe_config->adjusted_mode, mode);
> + pipe_config->adjusted_mode.base.id = 0;
> + drm_mode_copy(&pipe_config->requested_mode, mode);
> + pipe_config->requested_mode.base.id = 0;
drm_mode_copy() preserves the destination mode's id, and since you
kzalloc() the whole thing there's no need to zero the ids explicitly.
That was pretty much why I suggeted it.
The rest looks good to me.
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
--
Ville Syrjälä
Intel OTC
next prev parent reply other threads:[~2013-02-22 10:23 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ä [this message]
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
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=20130222102336.GK4469@intel.com \
--to=ville.syrjala@linux.intel.com \
--cc=daniel.vetter@ffwll.ch \
--cc=intel-gfx@lists.freedesktop.org \
/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.