Re: [PATCH] drm/i915: Move number of scalers initialization to runtime init

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/25/2016 3:14 PM, Chris Wilson wrote:
On Fri, Nov 25, 2016 at 03:16:58PM +0530, Nabendu Maiti wrote:
In future patches, we require greater flexibility in describing
the number of scalers available on each CRTC. To ease that transition
we move the current assignment to intel_device_info.

Scaler structure initialisation is done if scaler is available on the CRTC.
Gen9 check is not required as on depending upon numbers of scalers we
initialize scalers or return without doing anything in skl_init_scalers.

v2: Added Chris's commenents.
comments :)
Accepted. :)

Signed-off-by: Nabendu Maiti <nabendu.bikash.maiti@xxxxxxxxx>
Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>

---
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 drivers/gpu/drm/i915/intel_device_info.c |  3 +++
 drivers/gpu/drm/i915/intel_display.c     | 18 ++++++++----------
 3 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1ec9619..bb8c5f0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -758,6 +758,7 @@ struct intel_device_info {
 	u16 device_id;
 	u8 num_pipes;
 	u8 num_sprites[I915_MAX_PIPES];
+	u8 num_scalers[I915_MAX_PIPES];
 	u8 gen;
 	u16 gen_mask;
 	u8 ring_mask; /* Rings supported by the HW */
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 185e3bb..ef26fa8 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -282,6 +282,9 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 		info->num_sprites[PIPE_A] = 2;
 		info->num_sprites[PIPE_B] = 2;
 		info->num_sprites[PIPE_C] = 1;
+		info->num_scalers[PIPE_A] = 2;
+		info->num_scalers[PIPE_B] = 2;
+		info->num_scalers[PIPE_C] = 1;
 	} else if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) {
 		for_each_pipe(dev_priv, pipe)
 			info->num_sprites[pipe] = 2;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5d11002..2062170 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15266,6 +15266,11 @@ static void skl_init_scalers(struct drm_i915_private *dev_priv,
 		&crtc_state->scaler_state;
 	int i;

+	crtc->num_scalers = dev_priv->info.num_scalers[crtc->pipe];
+

Blank here is overkill

+	if (!crtc->num_scalers)
+		return;
+

crtc->num_scalers = <info>;
if (!ctrc->num_scalers)
	return;

is quite clean.
Accepted.


 	for (i = 0; i < crtc->num_scalers; i++) {
 		struct intel_scaler *scaler = &scaler_state->scalers[i];

@@ -15297,16 +15302,6 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 	intel_crtc->base.state = &crtc_state->base;
 	crtc_state->base.crtc = &intel_crtc->base;

-	/* initialize shared scalers */
-	if (INTEL_GEN(dev_priv) >= 9) {
-		if (pipe == PIPE_C)
-			intel_crtc->num_scalers = 1;
-		else
-			intel_crtc->num_scalers = SKL_NUM_SCALERS;
-
-		skl_init_scalers(dev_priv, intel_crtc, crtc_state);
-	}
-
 	primary = intel_primary_plane_create(dev_priv, pipe);
 	if (IS_ERR(primary)) {
 		ret = PTR_ERR(primary);
@@ -15348,6 +15343,9 @@ static int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)

 	intel_crtc->wm.cxsr_allowed = true;

+	/* initialize shared scalers */
+	skl_init_scalers(dev_priv, intel_crtc, crtc_state);

As this is now called by all, it really should be
intel_crtc_init_scalers() and we only need to pass intel_crtc at this
point.
okey , changing to intel_crtc_init_scalers()

I think we can also potspone it later on by substituting skl_* by intel_crtc_* or i9xx_crtc_*scaler in next patch. Cleanup needed for skylake_pfit** and some other scaler functions.

Yes removing dev_priv as folllowing,
struct drm_i915_private *dev_priv =  to_i915(crtc->base.dev); ?

But crtc_state need to to be passed, Apart from crtc_init , haswell_get_pipe_config also call it. We pass the current active pipe_state. Not sure *config in intel_crtc will serve the same purpose for both cases.

-Chris


--
Regards,
Nabendu
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux