Re: [Intel-gfx] [PATCH 11/20] drm/i915: prepare scaler for YCBCR420 modeset

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

 



On Thu, Jul 13, 2017 at 06:27:04PM +0530, Sharma, Shashank wrote:
> Regards
> 
> Shashank
> 
> 
> On 7/13/2017 6:22 PM, Ville Syrjälä wrote:
> > On Thu, Jul 13, 2017 at 10:51:04AM +0530, Sharma, Shashank wrote:
> >> Regards
> >>
> >> Shashank
> >>
> >>
> >> On 7/12/2017 10:47 PM, Ville Syrjälä wrote:
> >>> On Mon, Jul 10, 2017 at 04:48:39PM +0530, Shashank Sharma wrote:
> >>>> To get a YCBCR420 output from intel platforms, we need one
> >>>> scaler to scale down YCBCR444 samples to YCBCR420 samples.
> >>>>
> >>>> This patch:
> >>>> - Does scaler allocation for HDMI ycbcr420 outputs.
> >>>> - Programs PIPE_MISC register for ycbcr420 output.
> >>>> - Adds a new scaler user "HDMI output" to plug-into existing
> >>>>     scaler framework. This output type is identified using bit
> >>>>     30 of the scaler users bitmap.
> >>>>
> >>>> V2: rebase
> >>>> V3: rebase
> >>>> V4: rebase
> >>>> V5: addressed review comments from Ander:
> >>>>       - No need to check both scaler_user && hdmi_output.
> >>>>         Check for scaler_user is enough.
> >>>> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx>
> >>>> ---
> >>>>    drivers/gpu/drm/i915/intel_atomic.c  |  6 ++++++
> >>>>    drivers/gpu/drm/i915/intel_display.c | 23 +++++++++++++++++++++++
> >>>>    drivers/gpu/drm/i915/intel_drv.h     | 10 +++++++++-
> >>>>    drivers/gpu/drm/i915/intel_hdmi.c    | 12 ++++++++++++
> >>>>    drivers/gpu/drm/i915/intel_panel.c   |  3 ++-
> >>>>    5 files changed, 52 insertions(+), 2 deletions(-)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
> >>>> index 36d4e63..040d111 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_atomic.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_atomic.c
> >>>> @@ -264,6 +264,12 @@ int intel_atomic_setup_scalers(struct drm_i915_private *dev_priv,
> >>>>    
> >>>>    			/* panel fitter case: assign as a crtc scaler */
> >>>>    			scaler_id = &scaler_state->scaler_id;
> >>>> +		} else if (i == SKL_420_OUTPUT_INDEX) {
> >>> I think Ander already said that we should just tie this into the normal
> >>> pfit pipe scaler user. I agree. Having to users doesn't actually make
> >>> sense because there can be only one pipe scaler, and if we want to do
> >>> scaled 4:2:0 output then we anyway need to handle both cases
> >>> simultaneosly.
> >> I thought our idea of having a separate scalar user was a good one,
> >> because, this gives additional information
> >> how and why the scalar is being used. Also, we dont program the other
> >> scaling factors in this case, but we get
> >> the scaling profile set using PIPEMISC. So I would prefer to have this
> >> as a special case or separate user, but you
> >> can think about this, and let me know.
> > Eventually we'll want scaling and 420 at the same time. So a separate
> > user just doesn't work.
> Ok, I was just following the design, which we discussed during F2F. I 
> will remove this user then.

Yeah. Sorry for the ping-pong, I guess I didn't think this through
fully back then.

> - Shashank
> >> - Shashank
> >>>> +			name = "YCBCR420-OUTPUT";
> >>>> +			idx = intel_crtc->base.base.id;
> >>>> +
> >>>> +			/* YCBCR420 case: needs a pipe scaler */
> >>>> +			scaler_id = &scaler_state->scaler_id;
> >>>>    		} else {
> >>>>    			name = "PLANE";
> >>>>    
> >>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>>> index 01900e1..c56081e 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>>> @@ -4621,6 +4621,9 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
> >>>>    	 */
> >>>>    	need_scaling = src_w != dst_w || src_h != dst_h;
> >>>>    
> >>>> +	if (scaler_user == SKL_420_OUTPUT_INDEX)
> >>>> +		need_scaling = true;
> >>>> +
> >>>>    	/*
> >>>>    	 * if plane is being disabled or scaler is no more required or force detach
> >>>>    	 *  - free scaler binded to this plane/crtc
> >>>> @@ -4668,6 +4671,26 @@ skl_update_scaler(struct intel_crtc_state *crtc_state, bool force_detach,
> >>>>    }
> >>>>    
> >>>>    /**
> >>>> + * skl_update_scaler_crtc_420_output - Stages update to scaler state
> >>>> + * for YCBCR420 which needs a scaler, for downsampling.
> >>>> + *
> >>>> + * @state: crtc's scaler state
> >>>> + *
> >>>> + * Return
> >>>> + *     0 - scaler_usage updated successfully
> >>>> + *    error - requested scaling cannot be supported or other error condition
> >>>> + */
> >>>> +int skl_update_scaler_crtc_420_output(struct intel_crtc_state *state)
> >>>> +{
> >>>> +	const struct drm_display_mode *mode = &state->base.adjusted_mode;
> >>>> +
> >>>> +	return skl_update_scaler(state, !state->base.active,
> >>>> +		SKL_420_OUTPUT_INDEX, &state->scaler_state.scaler_id,
> >>>> +		state->pipe_src_w, state->pipe_src_h,
> >>>> +		mode->crtc_hdisplay, mode->crtc_vdisplay);
> >>>> +}
> >>>> +
> >>>> +/**
> >>>>     * skl_update_scaler_crtc - Stages update to scaler state for a given crtc.
> >>>>     *
> >>>>     * @state: crtc's scaler state
> >>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>>> index 592243b..68b4fba 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>>> @@ -471,7 +471,8 @@ struct intel_crtc_scaler_state {
> >>>>    	 *
> >>>>    	 *     If a bit is set, a user is using a scaler.
> >>>>    	 *     Here user can be a plane or crtc as defined below:
> >>>> -	 *       bits 0-30 - plane (bit position is index from drm_plane_index)
> >>>> +	 *       bits 0-29 - plane (bit position is index from drm_plane_index)
> >>>> +	 *       bit 30    - hdmi output
> >>>>    	 *       bit 31    - crtc
> >>>>    	 *
> >>>>    	 * Instead of creating a new index to cover planes and crtc, using
> >>>> @@ -484,6 +485,12 @@ struct intel_crtc_scaler_state {
> >>>>    	 * avilability.
> >>>>    	 */
> >>>>    #define SKL_CRTC_INDEX 31
> >>>> +
> >>>> +	/*
> >>>> +	 * YCBCR 420 output consume a scaler. So adding a user
> >>>> +	 * for 420 output requirement.
> >>>> +	 */
> >>>> +#define SKL_420_OUTPUT_INDEX 30
> >>>>    	unsigned scaler_users;
> >>>>    
> >>>>    	/* scaler used by crtc for panel fitting purpose */
> >>>> @@ -1483,6 +1490,7 @@ void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> >>>>    				 struct intel_crtc_state *pipe_config);
> >>>>    
> >>>>    int skl_update_scaler_crtc(struct intel_crtc_state *crtc_state);
> >>>> +int skl_update_scaler_crtc_420_output(struct intel_crtc_state *state);
> >>>>    int skl_max_scale(struct intel_crtc *crtc, struct intel_crtc_state *crtc_state);
> >>>>    
> >>>>    static inline u32 intel_plane_ggtt_offset(const struct intel_plane_state *state)
> >>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >>>> index 276d916..9e8d784 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>>> @@ -1355,6 +1355,7 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
> >>>>    			       struct intel_crtc_state *config,
> >>>>    			       int *clock_12bpc, int *clock_8bpc)
> >>>>    {
> >>>> +	struct intel_crtc *intel_crtc = to_intel_crtc(config->base.crtc);
> >>>>    
> >>>>    	if (!connector->ycbcr_420_allowed) {
> >>>>    		DRM_ERROR("Platform doesn't support YCBCR420 output\n");
> >>>> @@ -1366,6 +1367,17 @@ intel_hdmi_ycbcr420_config(struct drm_connector *connector,
> >>>>    	*clock_12bpc /= 2;
> >>>>    	*clock_8bpc /= 2;
> >>>>    	config->ycbcr420 = true;
> >>>> +
> >>>> +	/* YCBCR 420 output conversion needs a scaler */
> >>>> +	if (skl_update_scaler_crtc_420_output(config)) {
> >>>> +		DRM_ERROR("Scaler allocation for output failed\n");
> >>>> +		return false;
> >>>> +	}
> >>>> +
> >>>> +	/* Bind this scaler to pipe */
> >>>> +	intel_pch_panel_fitting(intel_crtc, config,
> >>>> +				DRM_MODE_SCALE_FULLSCREEN);
> >>>> +
> >>>>    	return true;
> >>>>    }
> >>>>    
> >>>> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> >>>> index 96c2cbd..fd2e081 100644
> >>>> --- a/drivers/gpu/drm/i915/intel_panel.c
> >>>> +++ b/drivers/gpu/drm/i915/intel_panel.c
> >>>> @@ -110,7 +110,8 @@ intel_pch_panel_fitting(struct intel_crtc *intel_crtc,
> >>>>    
> >>>>    	/* Native modes don't need fitting */
> >>>>    	if (adjusted_mode->crtc_hdisplay == pipe_config->pipe_src_w &&
> >>>> -	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h)
> >>>> +	    adjusted_mode->crtc_vdisplay == pipe_config->pipe_src_h &&
> >>>> +	    !pipe_config->ycbcr420)
> >>>>    		goto done;
> >>>>    
> >>>>    	switch (fitting_mode) {
> >>>> -- 
> >>>> 2.7.4
> >>>>
> >>>> _______________________________________________
> >>>> Intel-gfx mailing list
> >>>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> >>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux