From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: add encoder get_config function v3 Date: Wed, 8 May 2013 13:39:40 +0300 Message-ID: <20130508103940.GG14974@intel.com> References: <1367951711-7611-1-git-send-email-jbarnes@virtuousgeek.org> <1367966321-9417-1-git-send-email-jbarnes@virtuousgeek.org> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 8FCE2E5D36 for ; Wed, 8 May 2013 03:39:43 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1367966321-9417-1-git-send-email-jbarnes@virtuousgeek.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jesse Barnes Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, May 07, 2013 at 03:38:41PM -0700, Jesse Barnes wrote: > We can use this for fetching encoder specific pipe_config state, like > mode flags, adjusted clock, etc. > = > Just used for mode flags atm, so we can check the pipe config state at > mode set time. > = > v2: get_config when checking hw state too > v3: fix DVO and LVDS mode flags (Ville) > get SDVO DTD for flag fetch (Ville) > = > Signed-off-by: Jesse Barnes > --- > drivers/gpu/drm/i915/intel_crt.c | 23 ++++++++++++++++++ > drivers/gpu/drm/i915/intel_display.c | 20 +++++++++++++--- > drivers/gpu/drm/i915/intel_dp.c | 23 ++++++++++++++++++ > drivers/gpu/drm/i915/intel_drv.h | 4 ++++ > drivers/gpu/drm/i915/intel_dvo.c | 21 +++++++++++++++++ > drivers/gpu/drm/i915/intel_hdmi.c | 23 ++++++++++++++++++ > drivers/gpu/drm/i915/intel_lvds.c | 26 ++++++++++++++++++++ > drivers/gpu/drm/i915/intel_sdvo.c | 43 ++++++++++++++++++++++++++++= ++++++ > 8 files changed, 180 insertions(+), 3 deletions(-) > = > --- a/drivers/gpu/drm/i915/intel_sdvo.c > +++ b/drivers/gpu/drm/i915/intel_sdvo.c > @@ -712,6 +712,13 @@ static bool intel_sdvo_set_timing(struct intel_sdvo = *intel_sdvo, u8 cmd, > intel_sdvo_set_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->par= t2)); > } > = > +static bool intel_sdvo_get_timing(struct intel_sdvo *intel_sdvo, u8 cmd, > + struct intel_sdvo_dtd *dtd) > +{ > + return intel_sdvo_get_value(intel_sdvo, cmd, &dtd->part1, sizeof(dtd->p= art1)) && > + intel_sdvo_get_value(intel_sdvo, cmd + 1, &dtd->part2, sizeof(dtd->par= t2)); > +} > + > static bool intel_sdvo_set_input_timing(struct intel_sdvo *intel_sdvo, > struct intel_sdvo_dtd *dtd) > { > @@ -726,6 +733,13 @@ static bool intel_sdvo_set_output_timing(struct inte= l_sdvo *intel_sdvo, > SDVO_CMD_SET_OUTPUT_TIMINGS_PART1, dtd); > } > = > +static bool intel_sdvo_get_output_timing(struct intel_sdvo *intel_sdvo, > + struct intel_sdvo_dtd *dtd) > +{ > + return intel_sdvo_get_timing(intel_sdvo, > + SDVO_CMD_GET_OUTPUT_TIMINGS_PART2, dtd); PART1 Oh and it looks like we actually feed adjusted_mode into the input timings, not the output timings. So I'm thinking we should use the input timings in get_config() too. > +} > + > static bool > intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo, > uint16_t clock, > @@ -1264,6 +1278,33 @@ static bool intel_sdvo_get_hw_state(struct intel_e= ncoder *encoder, > return true; > } > = > +static void intel_sdvo_get_config(struct intel_encoder *encoder, > + struct intel_crtc_config *pipe_config) > +{ > + struct intel_sdvo *intel_sdvo =3D to_intel_sdvo(&encoder->base); > + struct intel_sdvo_dtd dtd; > + u32 flags =3D 0; > + bool ret; > + > + ret =3D intel_sdvo_get_output_timing(intel_sdvo, &dtd); > + if (!ret) { > + DRM_DEBUG_DRIVER("failed to retrieve SDVO DTD\n"); > + return; > + } > + > + if (dtd.part2.dtd_flags & DTD_FLAG_HSYNC_POSITIVE) > + flags |=3D DRM_MODE_FLAG_PHSYNC; > + else > + flags |=3D DRM_MODE_FLAG_NHSYNC; > + > + if (dtd.part2.dtd_flags & DTD_FLAG_VSYNC_POSITIVE) > + flags |=3D DRM_MODE_FLAG_PVSYNC; > + else > + flags |=3D DRM_MODE_FLAG_NVSYNC; > + > + pipe_config->adjusted_mode.flags |=3D flags; > +} > + > static void intel_disable_sdvo(struct intel_encoder *encoder) > { > struct drm_i915_private *dev_priv =3D encoder->base.dev->dev_private; > @@ -2793,6 +2834,8 @@ bool intel_sdvo_init(struct drm_device *dev, uint32= _t sdvo_reg, bool is_sdvob) > intel_encoder->mode_set =3D intel_sdvo_mode_set; > intel_encoder->enable =3D intel_enable_sdvo; > intel_encoder->get_hw_state =3D intel_sdvo_get_hw_state; > + if (INTEL_INFO(dev)->gen >=3D 4) > + intel_encoder->get_config =3D intel_sdvo_get_config; I'm assuming the gen4 check can be dropped now. So, as we discussed on irc, the only issue left is that we don't sanitize the adjusted_mode sync flags. I think they just get copied from the user specified mode directly, so if the user specifies neither + or - flag, or specifies both, intel_pipe_config_compare() will scream. > /* In default case sdvo lvds is false */ > if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps)) > -- = > 1.7.9.5 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC