[Intel-gfx] [PATCH 11/20] drm/i915: prepare scaler for YCBCR420 modeset
Sharma, Shashank
shashank.sharma at intel.com
Thu Jul 13 12:57:04 UTC 2017
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 at intel.com>
>>>> ---
>>>> 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.
- 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 at lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list