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