From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 197C7C4332F for ; Thu, 15 Dec 2022 09:12:52 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 53E4F10E507; Thu, 15 Dec 2022 09:12:50 +0000 (UTC) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTPS id D336710E507 for ; Thu, 15 Dec 2022 09:12:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1671095566; x=1702631566; h=from:to:subject:in-reply-to:references:date:message-id: mime-version:content-transfer-encoding; bh=6GF6Yds4RyBzyjDm+C53fDQ0oeIg3Kd4Hxf054LJE6A=; b=XwQ8PbMk7raDTCFSpqM5Ry1V6sZdjGku4Q2L1uKvMOPQbh3zi/O5NL3p zluR+8uvC8l5lpyNZXp8kv8MBunyh0maULbvI1+G2Lmlf+1nUEXIyC8FT iE1K3DYPtAW8MhoDzvOT/HqYzd4b6UwJpldGa4s0/zryB5l+WJTaffodx mbnu9gKCM+CKPJUkWeuOwsE5iuM87DYTmU3Oc8CCSTRSFhMG45H3idwSS Nh/Q9SiTJmeNN/Swx6EawJSNZYPL5J7xWHyuAqgGpB0vTdHqn9dMLZLt0 Ad4LGLRRw+g7ERJ5+eqx+rsjsGzZdwds0zBvSjpAb5D26ALpLxe0kOMCm A==; X-IronPort-AV: E=McAfee;i="6500,9779,10561"; a="306270628" X-IronPort-AV: E=Sophos;i="5.96,247,1665471600"; d="scan'208";a="306270628" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Dec 2022 01:12:46 -0800 X-IronPort-AV: E=McAfee;i="6500,9779,10561"; a="649303158" X-IronPort-AV: E=Sophos;i="5.96,247,1665471600"; d="scan'208";a="649303158" Received: from tgodea-mobl.ger.corp.intel.com (HELO localhost) ([10.252.61.26]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 15 Dec 2022 01:12:44 -0800 From: Jani Nikula To: "Murthy, Arun R" , "intel-gfx@lists.freedesktop.org" In-Reply-To: Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <87k02uwdlw.fsf@intel.com> Date: Thu, 15 Dec 2022 11:12:42 +0200 Message-ID: <87wn6tuj1x.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Intel-gfx] [PATCH 2/7] drm/i915/display: move more scanline functions to intel_vblank.[ch] X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" On Thu, 15 Dec 2022, "Murthy, Arun R" wrote: >> -----Original Message----- >> From: Nikula, Jani >> Sent: Wednesday, December 14, 2022 2:45 PM >> To: Murthy, Arun R ; intel- >> gfx@lists.freedesktop.org >> Subject: RE: [Intel-gfx] [PATCH 2/7] drm/i915/display: move more scanline >> functions to intel_vblank.[ch] >> >> On Wed, 14 Dec 2022, "Murthy, Arun R" wrote: >> >> -----Original Message----- >> >> From: Intel-gfx On Behalf >> >> Of Jani Nikula >> >> Sent: Monday, December 12, 2022 7:59 PM >> >> To: intel-gfx@lists.freedesktop.org >> >> Cc: Nikula, Jani >> >> Subject: [Intel-gfx] [PATCH 2/7] drm/i915/display: move more scanline >> >> functions to intel_vblank.[ch] >> >> >> >> Reduce clutter in intel_display.c by moving the scanline >> >> moving/stopped wait functions to intel_vblank.[ch]. >> >> >> >> Cc: Ville Syrj=C3=A4l=C3=A4 >> >> Signed-off-by: Jani Nikula >> >> --- >> >> drivers/gpu/drm/i915/display/intel_display.c | 36 >> >> +------------------- drivers/gpu/drm/i915/display/intel_vblank.c | >> >> 35 +++++++++++++++++++ drivers/gpu/drm/i915/display/intel_vblank.h | >> >> 2 ++ >> >> 3 files changed, 38 insertions(+), 35 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c >> >> b/drivers/gpu/drm/i915/display/intel_display.c >> >> index 6cdfdae2c712..0cdb514d7ee0 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_display.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c >> >> @@ -115,6 +115,7 @@ >> >> #include "intel_quirks.h" >> >> #include "intel_sprite.h" >> >> #include "intel_tc.h" >> >> +#include "intel_vblank.h" >> >> #include "intel_vga.h" >> >> #include "i9xx_plane.h" >> >> #include "skl_scaler.h" >> >> @@ -386,41 +387,6 @@ struct intel_crtc *intel_master_crtc(const >> >> struct intel_crtc_state *crtc_state) >> >> return to_intel_crtc(crtc_state->uapi.crtc); >> >> } >> >> >> >> -static bool pipe_scanline_is_moving(struct drm_i915_private *dev_pri= v, >> >> - enum pipe pipe) >> >> -{ >> >> - i915_reg_t reg =3D PIPEDSL(pipe); >> >> - u32 line1, line2; >> >> - >> >> - line1 =3D intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK; >> >> - msleep(5); >> >> - line2 =3D intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK; >> >> - >> >> - return line1 !=3D line2; >> >> -} >> >> - >> >> -static void wait_for_pipe_scanline_moving(struct intel_crtc *crtc, >> >> bool state) -{ >> >> - struct drm_i915_private *dev_priv =3D to_i915(crtc->base.dev); >> >> - enum pipe pipe =3D crtc->pipe; >> >> - >> >> - /* Wait for the display line to settle/start moving */ >> >> - if (wait_for(pipe_scanline_is_moving(dev_priv, pipe) =3D=3D sta= te, 100)) >> >> - drm_err(&dev_priv->drm, >> >> - "pipe %c scanline %s wait timed out\n", >> >> - pipe_name(pipe), str_on_off(state)); >> >> -} >> >> - >> >> -static void intel_wait_for_pipe_scanline_stopped(struct intel_crtc *= crtc) -{ >> >> - wait_for_pipe_scanline_moving(crtc, false); >> >> -} >> >> - >> >> -static void intel_wait_for_pipe_scanline_moving(struct intel_crtc *c= rtc) -{ >> >> - wait_for_pipe_scanline_moving(crtc, true); >> >> -} >> >> - >> >> static void >> >> intel_wait_for_pipe_off(const struct intel_crtc_state >> >> *old_crtc_state) { diff -- git >> >> a/drivers/gpu/drm/i915/display/intel_vblank.c >> >> b/drivers/gpu/drm/i915/display/intel_vblank.c >> >> index 78a579496ad1..f25ec643a0a3 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_vblank.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c >> >> @@ -417,3 +417,38 @@ int intel_get_crtc_scanline(struct intel_crtc >> >> *crtc) >> >> >> >> return position; >> >> } >> >> + >> >> +static bool pipe_scanline_is_moving(struct drm_i915_private *dev_pri= v, >> >> + enum pipe pipe) { >> >> + i915_reg_t reg =3D PIPEDSL(pipe); >> >> + u32 line1, line2; >> >> + >> >> + line1 =3D intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK; >> >> + msleep(5); >> >> + line2 =3D intel_de_read(dev_priv, reg) & PIPEDSL_LINE_MASK; >> >> + >> >> + return line1 !=3D line2; >> >> +} >> >> + >> >> +static void wait_for_pipe_scanline_moving(struct intel_crtc *crtc, >> >> +bool >> >> +state) { >> >> + struct drm_i915_private *dev_priv =3D to_i915(crtc->base.dev); >> >> + enum pipe pipe =3D crtc->pipe; >> >> + >> >> + /* Wait for the display line to settle/start moving */ >> >> + if (wait_for(pipe_scanline_is_moving(dev_priv, pipe) =3D=3D sta= te, 100)) >> >> + drm_err(&dev_priv->drm, >> >> + "pipe %c scanline %s wait timed out\n", >> >> + pipe_name(pipe), str_on_off(state)); } >> >> + >> >> +void intel_wait_for_pipe_scanline_stopped(struct intel_crtc *crtc) { >> >> + wait_for_pipe_scanline_moving(crtc, false); } >> >> + >> > Is this wrapper function required, since nothing else is being other >> > than calling the function wait_for_pipe_scanline_moving, can this be >> > replaced? >> >> Well, first, this patch is only about code *movement*. Any changes like = that >> would have to be separate. >> > Ok. > Reviewed-by: Arun R Murthy > >> And how would you propose to drop the wrapper? The wrappers are all >> about readability in the caller side: >> > I didn=E2=80=99t mean to drop the wrapper, understand that wrapper is mor= e readable, what I meant is to replace wait_for_pipe_scanline_moving/stoppe= d with its function contents. Why should we duplicate the code? BR, Jani. > > Thanks and Regards, > Arun R Murthy > -------------------- >> intel_wait_for_pipe_scanline_stopped(crtc) >> >> vs. >> >> intel_wait_for_pipe_scanline_moving(crtc, false) >> >> BR, >> Jani. >> >> > >> > Thanks and Regards, >> > Arun R Murthy >> > -------------------- >> > >> >> +void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc) { >> >> + wait_for_pipe_scanline_moving(crtc, true); } >> >> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.h >> >> b/drivers/gpu/drm/i915/display/intel_vblank.h >> >> index 9c0034d7454d..54870cabd734 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_vblank.h >> >> +++ b/drivers/gpu/drm/i915/display/intel_vblank.h >> >> @@ -17,5 +17,7 @@ u32 g4x_get_vblank_counter(struct drm_crtc *crtc); >> >> bool intel_crtc_get_vblank_timestamp(struct drm_crtc *crtc, int >> *max_error, >> >> ktime_t *vblank_time, bool >> >> in_vblank_irq); int intel_get_crtc_scanline(struct intel_crtc *crtc); >> >> +void intel_wait_for_pipe_scanline_stopped(struct intel_crtc *crtc); >> >> +void intel_wait_for_pipe_scanline_moving(struct intel_crtc *crtc); >> >> >> >> #endif /* __INTEL_VBLANK_H__ */ >> >> -- >> >> 2.34.1 >> > >> >> -- >> Jani Nikula, Intel Open Source Graphics Center --=20 Jani Nikula, Intel Open Source Graphics Center