| Message ID | 20181112170000.27531-16-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix TV encoder support | expand |
On Mon, Nov 12, 2018 at 06:59:59PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > On gen3 we must disable the TV encoder vertical filter for >1024 > pixel wide sources. Once that's done all we can is try to center > the image on the screen. Naturally the TV mode vertical resolution > must be equal or larger than the user mode vertical resolution > or else we'd have to cut off part of the user mode. > > And while we may not be able to respect the user's choice of > top and bottom borders exactly (or we'd have to reject he mode > most likely), we can try to maintain the relative sizes of the > top and bottom border with respect to each orher. > > Additionally we must configure the pipe as interlaced if the > TV mode is interlaced. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_tv.c | 100 +++++++++++++++++++++++++++++--- > 1 file changed, 92 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c > index 75126fce655d..7099d837e31a 100644 > --- a/drivers/gpu/drm/i915/intel_tv.c > +++ b/drivers/gpu/drm/i915/intel_tv.c > @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = { > }, > }; > > +struct intel_tv_connector_state { > + struct drm_connector_state base; > + > + /* > + * May need to override the user margins for > + * gen3 >1024 wide source vertical centering. > + */ > + struct { > + u16 top, bottom; > + } margins; > + > + bool bypass_vfilter; > +}; > + > +#define to_intel_tv_connector_state(x) container_of(x, struct intel_tv_connector_state, base) > + > +/** > + * intel_digital_connector_duplicate_state - duplicate connector state ^intel_tv_connector_duplicate_state > + * @connector: digital connector ^tv connector? > + * > + * Allocates and returns a copy of the connector state (both common and > + * digital connector specific) for the specified connector. > + * > + * Returns: The newly allocated connector state, or NULL on failure. > + */ > +struct drm_connector_state * > +intel_tv_connector_duplicate_state(struct drm_connector *connector) > +{ > + struct intel_tv_connector_state *state; > + > + state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL); > + if (!state) > + return NULL; > + > + __drm_atomic_helper_connector_duplicate_state(connector, &state->base); > + return &state->base; > +} You didn't add the corresponding checks for the new intel_tv_connector_state fields to intel_tv_atomic_check(). I suppose that's ok since something resulting in a change in those will force a modeset anyway: Reviewed-by: Imre Deak <imre.deak@intel.com> > + > static struct intel_tv *enc_to_tv(struct intel_encoder *encoder) > { > return container_of(encoder, struct intel_tv, base); > @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder, > struct intel_crtc_state *pipe_config, > struct drm_connector_state *conn_state) > { > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > + struct intel_tv_connector_state *tv_conn_state = > + to_intel_tv_connector_state(conn_state); > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); > struct drm_display_mode *adjusted_mode = > &pipe_config->base.adjusted_mode; > @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder *encoder, > pipe_config->port_clock = tv_mode->clock; > > intel_tv_mode_to_mode(adjusted_mode, tv_mode); > + drm_mode_set_crtcinfo(adjusted_mode, 0); > + > + if (IS_GEN3(dev_priv) && hdisplay > 1024) { > + int extra, top, bottom; > + > + extra = adjusted_mode->crtc_vdisplay - vdisplay; > + > + if (extra < 0) { > + DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide modes\n"); > + return false; > + } > + > + /* Need to turn off the vertical filter and center the image */ > + > + /* Attempt to maintain the relative sizes of the margins */ > + top = conn_state->tv.margins.top; > + bottom = conn_state->tv.margins.bottom; > + > + if (top + bottom) > + top = extra * top / (top + bottom); > + else > + top = extra / 2; > + bottom = extra - top; > + > + tv_conn_state->margins.top = top; > + tv_conn_state->margins.bottom = bottom; > + > + tv_conn_state->bypass_vfilter = true; > + > + if (!tv_mode->progressive) > + adjusted_mode->flags |= DRM_MODE_FLAG_INTERLACE; > + } else { > + tv_conn_state->margins.top = conn_state->tv.margins.top; > + tv_conn_state->margins.bottom = conn_state->tv.margins.bottom; > + > + tv_conn_state->bypass_vfilter = false; > + } > > DRM_DEBUG_KMS("TV mode:\n"); > drm_mode_debug_printmodeline(adjusted_mode); > @@ -1222,8 +1300,8 @@ intel_tv_compute_config(struct intel_encoder *encoder, > conn_state->tv.margins.left, > conn_state->tv.margins.right); > intel_tv_scale_mode_vert(adjusted_mode, vdisplay, > - conn_state->tv.margins.top, > - conn_state->tv.margins.bottom); > + tv_conn_state->margins.top, > + tv_conn_state->margins.bottom); > drm_mode_set_crtcinfo(adjusted_mode, 0); > adjusted_mode->name[0] = '\0'; > > @@ -1316,8 +1394,10 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); > struct intel_tv *intel_tv = enc_to_tv(encoder); > + const struct intel_tv_connector_state *tv_conn_state = > + to_intel_tv_connector_state(conn_state); > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); > - u32 tv_ctl; > + u32 tv_ctl, tv_filter_ctl; > u32 scctl1, scctl2, scctl3; > int i, j; > const struct video_levels *video_levels; > @@ -1425,16 +1505,20 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, > assert_pipe_disabled(dev_priv, intel_crtc->pipe); > > /* Filter ctl must be set before TV_WIN_SIZE */ > - I915_WRITE(TV_FILTER_CTL_1, TV_AUTO_SCALE); > + tv_filter_ctl = TV_AUTO_SCALE; > + if (tv_conn_state->bypass_vfilter) > + tv_filter_ctl |= TV_V_FILTER_BYPASS; > + I915_WRITE(TV_FILTER_CTL_1, tv_filter_ctl); > + > xsize = tv_mode->hblank_start - tv_mode->hblank_end; > ysize = intel_tv_mode_vdisplay(tv_mode); > > xpos = conn_state->tv.margins.left; > - ypos = conn_state->tv.margins.top; > + ypos = tv_conn_state->margins.top; > xsize -= (conn_state->tv.margins.left + > conn_state->tv.margins.right); > - ysize -= (conn_state->tv.margins.top + > - conn_state->tv.margins.bottom); > + ysize -= (tv_conn_state->margins.top + > + tv_conn_state->margins.bottom); > I915_WRITE(TV_WIN_POS, (xpos<<16)|ypos); > I915_WRITE(TV_WIN_SIZE, (xsize<<16)|ysize); > > @@ -1701,7 +1785,7 @@ static const struct drm_connector_funcs intel_tv_connector_funcs = { > .destroy = intel_connector_destroy, > .fill_modes = drm_helper_probe_single_connector_modes, > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > + .atomic_duplicate_state = intel_tv_connector_duplicate_state, > }; > > static int intel_tv_atomic_check(struct drm_connector *connector, > -- > 2.18.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Jan 23, 2019 at 03:49:02PM +0200, Imre Deak wrote: > On Mon, Nov 12, 2018 at 06:59:59PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > On gen3 we must disable the TV encoder vertical filter for >1024 > > pixel wide sources. Once that's done all we can is try to center > > the image on the screen. Naturally the TV mode vertical resolution > > must be equal or larger than the user mode vertical resolution > > or else we'd have to cut off part of the user mode. > > > > And while we may not be able to respect the user's choice of > > top and bottom borders exactly (or we'd have to reject he mode > > most likely), we can try to maintain the relative sizes of the > > top and bottom border with respect to each orher. > > > > Additionally we must configure the pipe as interlaced if the > > TV mode is interlaced. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_tv.c | 100 +++++++++++++++++++++++++++++--- > > 1 file changed, 92 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c > > index 75126fce655d..7099d837e31a 100644 > > --- a/drivers/gpu/drm/i915/intel_tv.c > > +++ b/drivers/gpu/drm/i915/intel_tv.c > > @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = { > > }, > > }; > > > > +struct intel_tv_connector_state { > > + struct drm_connector_state base; > > + > > + /* > > + * May need to override the user margins for > > + * gen3 >1024 wide source vertical centering. > > + */ > > + struct { > > + u16 top, bottom; > > + } margins; > > + > > + bool bypass_vfilter; > > +}; > > + > > +#define to_intel_tv_connector_state(x) container_of(x, struct intel_tv_connector_state, base) > > + > > +/** > > + * intel_digital_connector_duplicate_state - duplicate connector state > ^intel_tv_connector_duplicate_state > > + * @connector: digital connector > ^tv connector? > > + * > > + * Allocates and returns a copy of the connector state (both common and > > + * digital connector specific) for the specified connector. > > + * > > + * Returns: The newly allocated connector state, or NULL on failure. > > + */ > > +struct drm_connector_state * > > +intel_tv_connector_duplicate_state(struct drm_connector *connector) > > +{ > > + struct intel_tv_connector_state *state; > > + > > + state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL); > > + if (!state) > > + return NULL; > > + > > + __drm_atomic_helper_connector_duplicate_state(connector, &state->base); > > + return &state->base; > > +} > > You didn't add the corresponding checks for the new > intel_tv_connector_state fields to intel_tv_atomic_check(). I suppose > that's ok since something resulting in a change in those will force a > modeset anyway: The new fields are not visible to the user so nothing external will change them. intel_tv_compute_config() (which is executed after .atomic_check()) will just recompute them based on other user visible state. > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > + > > static struct intel_tv *enc_to_tv(struct intel_encoder *encoder) > > { > > return container_of(encoder, struct intel_tv, base); > > @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > struct intel_crtc_state *pipe_config, > > struct drm_connector_state *conn_state) > > { > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_tv_connector_state *tv_conn_state = > > + to_intel_tv_connector_state(conn_state); > > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); > > struct drm_display_mode *adjusted_mode = > > &pipe_config->base.adjusted_mode; > > @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > pipe_config->port_clock = tv_mode->clock; > > > > intel_tv_mode_to_mode(adjusted_mode, tv_mode); > > + drm_mode_set_crtcinfo(adjusted_mode, 0); > > + > > + if (IS_GEN3(dev_priv) && hdisplay > 1024) { > > + int extra, top, bottom; > > + > > + extra = adjusted_mode->crtc_vdisplay - vdisplay; > > + > > + if (extra < 0) { > > + DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide modes\n"); > > + return false; > > + } > > + > > + /* Need to turn off the vertical filter and center the image */ > > + > > + /* Attempt to maintain the relative sizes of the margins */ > > + top = conn_state->tv.margins.top; > > + bottom = conn_state->tv.margins.bottom; > > + > > + if (top + bottom) > > + top = extra * top / (top + bottom); > > + else > > + top = extra / 2; > > + bottom = extra - top; > > + > > + tv_conn_state->margins.top = top; > > + tv_conn_state->margins.bottom = bottom; > > + > > + tv_conn_state->bypass_vfilter = true; > > + > > + if (!tv_mode->progressive) > > + adjusted_mode->flags |= DRM_MODE_FLAG_INTERLACE; > > + } else { > > + tv_conn_state->margins.top = conn_state->tv.margins.top; > > + tv_conn_state->margins.bottom = conn_state->tv.margins.bottom; > > + > > + tv_conn_state->bypass_vfilter = false; > > + } > > > > DRM_DEBUG_KMS("TV mode:\n"); > > drm_mode_debug_printmodeline(adjusted_mode); > > @@ -1222,8 +1300,8 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > conn_state->tv.margins.left, > > conn_state->tv.margins.right); > > intel_tv_scale_mode_vert(adjusted_mode, vdisplay, > > - conn_state->tv.margins.top, > > - conn_state->tv.margins.bottom); > > + tv_conn_state->margins.top, > > + tv_conn_state->margins.bottom); > > drm_mode_set_crtcinfo(adjusted_mode, 0); > > adjusted_mode->name[0] = '\0'; > > > > @@ -1316,8 +1394,10 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); > > struct intel_tv *intel_tv = enc_to_tv(encoder); > > + const struct intel_tv_connector_state *tv_conn_state = > > + to_intel_tv_connector_state(conn_state); > > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); > > - u32 tv_ctl; > > + u32 tv_ctl, tv_filter_ctl; > > u32 scctl1, scctl2, scctl3; > > int i, j; > > const struct video_levels *video_levels; > > @@ -1425,16 +1505,20 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, > > assert_pipe_disabled(dev_priv, intel_crtc->pipe); > > > > /* Filter ctl must be set before TV_WIN_SIZE */ > > - I915_WRITE(TV_FILTER_CTL_1, TV_AUTO_SCALE); > > + tv_filter_ctl = TV_AUTO_SCALE; > > + if (tv_conn_state->bypass_vfilter) > > + tv_filter_ctl |= TV_V_FILTER_BYPASS; > > + I915_WRITE(TV_FILTER_CTL_1, tv_filter_ctl); > > + > > xsize = tv_mode->hblank_start - tv_mode->hblank_end; > > ysize = intel_tv_mode_vdisplay(tv_mode); > > > > xpos = conn_state->tv.margins.left; > > - ypos = conn_state->tv.margins.top; > > + ypos = tv_conn_state->margins.top; > > xsize -= (conn_state->tv.margins.left + > > conn_state->tv.margins.right); > > - ysize -= (conn_state->tv.margins.top + > > - conn_state->tv.margins.bottom); > > + ysize -= (tv_conn_state->margins.top + > > + tv_conn_state->margins.bottom); > > I915_WRITE(TV_WIN_POS, (xpos<<16)|ypos); > > I915_WRITE(TV_WIN_SIZE, (xsize<<16)|ysize); > > > > @@ -1701,7 +1785,7 @@ static const struct drm_connector_funcs intel_tv_connector_funcs = { > > .destroy = intel_connector_destroy, > > .fill_modes = drm_helper_probe_single_connector_modes, > > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > > + .atomic_duplicate_state = intel_tv_connector_duplicate_state, > > }; > > > > static int intel_tv_atomic_check(struct drm_connector *connector, > > -- > > 2.18.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Jan 23, 2019 at 06:38:17PM +0200, Ville Syrjälä wrote: > On Wed, Jan 23, 2019 at 03:49:02PM +0200, Imre Deak wrote: > > On Mon, Nov 12, 2018 at 06:59:59PM +0200, Ville Syrjala wrote: > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > > > On gen3 we must disable the TV encoder vertical filter for >1024 > > > pixel wide sources. Once that's done all we can is try to center > > > the image on the screen. Naturally the TV mode vertical resolution > > > must be equal or larger than the user mode vertical resolution > > > or else we'd have to cut off part of the user mode. > > > > > > And while we may not be able to respect the user's choice of > > > top and bottom borders exactly (or we'd have to reject he mode > > > most likely), we can try to maintain the relative sizes of the > > > top and bottom border with respect to each orher. > > > > > > Additionally we must configure the pipe as interlaced if the > > > TV mode is interlaced. > > > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > --- > > > drivers/gpu/drm/i915/intel_tv.c | 100 +++++++++++++++++++++++++++++--- > > > 1 file changed, 92 insertions(+), 8 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c > > > index 75126fce655d..7099d837e31a 100644 > > > --- a/drivers/gpu/drm/i915/intel_tv.c > > > +++ b/drivers/gpu/drm/i915/intel_tv.c > > > @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = { > > > }, > > > }; > > > > > > +struct intel_tv_connector_state { > > > + struct drm_connector_state base; > > > + > > > + /* > > > + * May need to override the user margins for > > > + * gen3 >1024 wide source vertical centering. > > > + */ > > > + struct { > > > + u16 top, bottom; > > > + } margins; > > > + > > > + bool bypass_vfilter; > > > +}; > > > + > > > +#define to_intel_tv_connector_state(x) container_of(x, struct intel_tv_connector_state, base) > > > + > > > +/** > > > + * intel_digital_connector_duplicate_state - duplicate connector state > > ^intel_tv_connector_duplicate_state > > > + * @connector: digital connector > > ^tv connector? > > > + * > > > + * Allocates and returns a copy of the connector state (both common and > > > + * digital connector specific) for the specified connector. > > > + * > > > + * Returns: The newly allocated connector state, or NULL on failure. > > > + */ > > > +struct drm_connector_state * > > > +intel_tv_connector_duplicate_state(struct drm_connector *connector) > > > +{ > > > + struct intel_tv_connector_state *state; > > > + > > > + state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL); > > > + if (!state) > > > + return NULL; > > > + > > > + __drm_atomic_helper_connector_duplicate_state(connector, &state->base); > > > + return &state->base; > > > +} > > > > You didn't add the corresponding checks for the new > > intel_tv_connector_state fields to intel_tv_atomic_check(). I suppose > > that's ok since something resulting in a change in those will force a > > modeset anyway: > > The new fields are not visible to the user so nothing external > will change them. intel_tv_compute_config() (which is executed > after .atomic_check()) will just recompute them based on other > user visible state. Ah, right, that explains and I missed the fact that it makes no sense to check any params that are computed :/ > > > > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > > > + > > > static struct intel_tv *enc_to_tv(struct intel_encoder *encoder) > > > { > > > return container_of(encoder, struct intel_tv, base); > > > @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > > struct intel_crtc_state *pipe_config, > > > struct drm_connector_state *conn_state) > > > { > > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > > + struct intel_tv_connector_state *tv_conn_state = > > > + to_intel_tv_connector_state(conn_state); > > > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); > > > struct drm_display_mode *adjusted_mode = > > > &pipe_config->base.adjusted_mode; > > > @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > > pipe_config->port_clock = tv_mode->clock; > > > > > > intel_tv_mode_to_mode(adjusted_mode, tv_mode); > > > + drm_mode_set_crtcinfo(adjusted_mode, 0); > > > + > > > + if (IS_GEN3(dev_priv) && hdisplay > 1024) { > > > + int extra, top, bottom; > > > + > > > + extra = adjusted_mode->crtc_vdisplay - vdisplay; > > > + > > > + if (extra < 0) { > > > + DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide modes\n"); > > > + return false; > > > + } > > > + > > > + /* Need to turn off the vertical filter and center the image */ > > > + > > > + /* Attempt to maintain the relative sizes of the margins */ > > > + top = conn_state->tv.margins.top; > > > + bottom = conn_state->tv.margins.bottom; > > > + > > > + if (top + bottom) > > > + top = extra * top / (top + bottom); > > > + else > > > + top = extra / 2; > > > + bottom = extra - top; > > > + > > > + tv_conn_state->margins.top = top; > > > + tv_conn_state->margins.bottom = bottom; > > > + > > > + tv_conn_state->bypass_vfilter = true; > > > + > > > + if (!tv_mode->progressive) > > > + adjusted_mode->flags |= DRM_MODE_FLAG_INTERLACE; > > > + } else { > > > + tv_conn_state->margins.top = conn_state->tv.margins.top; > > > + tv_conn_state->margins.bottom = conn_state->tv.margins.bottom; > > > + > > > + tv_conn_state->bypass_vfilter = false; > > > + } > > > > > > DRM_DEBUG_KMS("TV mode:\n"); > > > drm_mode_debug_printmodeline(adjusted_mode); > > > @@ -1222,8 +1300,8 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > > conn_state->tv.margins.left, > > > conn_state->tv.margins.right); > > > intel_tv_scale_mode_vert(adjusted_mode, vdisplay, > > > - conn_state->tv.margins.top, > > > - conn_state->tv.margins.bottom); > > > + tv_conn_state->margins.top, > > > + tv_conn_state->margins.bottom); > > > drm_mode_set_crtcinfo(adjusted_mode, 0); > > > adjusted_mode->name[0] = '\0'; > > > > > > @@ -1316,8 +1394,10 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, > > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); > > > struct intel_tv *intel_tv = enc_to_tv(encoder); > > > + const struct intel_tv_connector_state *tv_conn_state = > > > + to_intel_tv_connector_state(conn_state); > > > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); > > > - u32 tv_ctl; > > > + u32 tv_ctl, tv_filter_ctl; > > > u32 scctl1, scctl2, scctl3; > > > int i, j; > > > const struct video_levels *video_levels; > > > @@ -1425,16 +1505,20 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, > > > assert_pipe_disabled(dev_priv, intel_crtc->pipe); > > > > > > /* Filter ctl must be set before TV_WIN_SIZE */ > > > - I915_WRITE(TV_FILTER_CTL_1, TV_AUTO_SCALE); > > > + tv_filter_ctl = TV_AUTO_SCALE; > > > + if (tv_conn_state->bypass_vfilter) > > > + tv_filter_ctl |= TV_V_FILTER_BYPASS; > > > + I915_WRITE(TV_FILTER_CTL_1, tv_filter_ctl); > > > + > > > xsize = tv_mode->hblank_start - tv_mode->hblank_end; > > > ysize = intel_tv_mode_vdisplay(tv_mode); > > > > > > xpos = conn_state->tv.margins.left; > > > - ypos = conn_state->tv.margins.top; > > > + ypos = tv_conn_state->margins.top; > > > xsize -= (conn_state->tv.margins.left + > > > conn_state->tv.margins.right); > > > - ysize -= (conn_state->tv.margins.top + > > > - conn_state->tv.margins.bottom); > > > + ysize -= (tv_conn_state->margins.top + > > > + tv_conn_state->margins.bottom); > > > I915_WRITE(TV_WIN_POS, (xpos<<16)|ypos); > > > I915_WRITE(TV_WIN_SIZE, (xsize<<16)|ysize); > > > > > > @@ -1701,7 +1785,7 @@ static const struct drm_connector_funcs intel_tv_connector_funcs = { > > > .destroy = intel_connector_destroy, > > > .fill_modes = drm_helper_probe_single_connector_modes, > > > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > > > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > > > + .atomic_duplicate_state = intel_tv_connector_duplicate_state, > > > }; > > > > > > static int intel_tv_atomic_check(struct drm_connector *connector, > > > -- > > > 2.18.1 > > > > > > _______________________________________________ > > > Intel-gfx mailing list > > > Intel-gfx@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > -- > Ville Syrjälä > Intel
On Wed, Jan 23, 2019 at 03:49:02PM +0200, Imre Deak wrote: > On Mon, Nov 12, 2018 at 06:59:59PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > On gen3 we must disable the TV encoder vertical filter for >1024 > > pixel wide sources. Once that's done all we can is try to center > > the image on the screen. Naturally the TV mode vertical resolution > > must be equal or larger than the user mode vertical resolution > > or else we'd have to cut off part of the user mode. > > > > And while we may not be able to respect the user's choice of > > top and bottom borders exactly (or we'd have to reject he mode > > most likely), we can try to maintain the relative sizes of the > > top and bottom border with respect to each orher. > > > > Additionally we must configure the pipe as interlaced if the > > TV mode is interlaced. > > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_tv.c | 100 +++++++++++++++++++++++++++++--- > > 1 file changed, 92 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c > > index 75126fce655d..7099d837e31a 100644 > > --- a/drivers/gpu/drm/i915/intel_tv.c > > +++ b/drivers/gpu/drm/i915/intel_tv.c > > @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = { > > }, > > }; > > > > +struct intel_tv_connector_state { > > + struct drm_connector_state base; > > + > > + /* > > + * May need to override the user margins for > > + * gen3 >1024 wide source vertical centering. > > + */ > > + struct { > > + u16 top, bottom; > > + } margins; > > + > > + bool bypass_vfilter; > > +}; > > + > > +#define to_intel_tv_connector_state(x) container_of(x, struct intel_tv_connector_state, base) > > + > > +/** > > + * intel_digital_connector_duplicate_state - duplicate connector state > ^intel_tv_connector_duplicate_state > > + * @connector: digital connector > ^tv connector? Copy paste fail. I just dropped the kerneldoc here and made the function static. Also fixed up the s/IS_GEN3(x)/IS_GEN(x,3)/ type of things and pushed the lot to dinq. Thanks for the review. > > + * > > + * Allocates and returns a copy of the connector state (both common and > > + * digital connector specific) for the specified connector. > > + * > > + * Returns: The newly allocated connector state, or NULL on failure. > > + */ > > +struct drm_connector_state * > > +intel_tv_connector_duplicate_state(struct drm_connector *connector) > > +{ > > + struct intel_tv_connector_state *state; > > + > > + state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL); > > + if (!state) > > + return NULL; > > + > > + __drm_atomic_helper_connector_duplicate_state(connector, &state->base); > > + return &state->base; > > +} > > You didn't add the corresponding checks for the new > intel_tv_connector_state fields to intel_tv_atomic_check(). I suppose > that's ok since something resulting in a change in those will force a > modeset anyway: > > Reviewed-by: Imre Deak <imre.deak@intel.com> > > > + > > static struct intel_tv *enc_to_tv(struct intel_encoder *encoder) > > { > > return container_of(encoder, struct intel_tv, base); > > @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > struct intel_crtc_state *pipe_config, > > struct drm_connector_state *conn_state) > > { > > + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > + struct intel_tv_connector_state *tv_conn_state = > > + to_intel_tv_connector_state(conn_state); > > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); > > struct drm_display_mode *adjusted_mode = > > &pipe_config->base.adjusted_mode; > > @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > pipe_config->port_clock = tv_mode->clock; > > > > intel_tv_mode_to_mode(adjusted_mode, tv_mode); > > + drm_mode_set_crtcinfo(adjusted_mode, 0); > > + > > + if (IS_GEN3(dev_priv) && hdisplay > 1024) { > > + int extra, top, bottom; > > + > > + extra = adjusted_mode->crtc_vdisplay - vdisplay; > > + > > + if (extra < 0) { > > + DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide modes\n"); > > + return false; > > + } > > + > > + /* Need to turn off the vertical filter and center the image */ > > + > > + /* Attempt to maintain the relative sizes of the margins */ > > + top = conn_state->tv.margins.top; > > + bottom = conn_state->tv.margins.bottom; > > + > > + if (top + bottom) > > + top = extra * top / (top + bottom); > > + else > > + top = extra / 2; > > + bottom = extra - top; > > + > > + tv_conn_state->margins.top = top; > > + tv_conn_state->margins.bottom = bottom; > > + > > + tv_conn_state->bypass_vfilter = true; > > + > > + if (!tv_mode->progressive) > > + adjusted_mode->flags |= DRM_MODE_FLAG_INTERLACE; > > + } else { > > + tv_conn_state->margins.top = conn_state->tv.margins.top; > > + tv_conn_state->margins.bottom = conn_state->tv.margins.bottom; > > + > > + tv_conn_state->bypass_vfilter = false; > > + } > > > > DRM_DEBUG_KMS("TV mode:\n"); > > drm_mode_debug_printmodeline(adjusted_mode); > > @@ -1222,8 +1300,8 @@ intel_tv_compute_config(struct intel_encoder *encoder, > > conn_state->tv.margins.left, > > conn_state->tv.margins.right); > > intel_tv_scale_mode_vert(adjusted_mode, vdisplay, > > - conn_state->tv.margins.top, > > - conn_state->tv.margins.bottom); > > + tv_conn_state->margins.top, > > + tv_conn_state->margins.bottom); > > drm_mode_set_crtcinfo(adjusted_mode, 0); > > adjusted_mode->name[0] = '\0'; > > > > @@ -1316,8 +1394,10 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, > > struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); > > struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); > > struct intel_tv *intel_tv = enc_to_tv(encoder); > > + const struct intel_tv_connector_state *tv_conn_state = > > + to_intel_tv_connector_state(conn_state); > > const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); > > - u32 tv_ctl; > > + u32 tv_ctl, tv_filter_ctl; > > u32 scctl1, scctl2, scctl3; > > int i, j; > > const struct video_levels *video_levels; > > @@ -1425,16 +1505,20 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, > > assert_pipe_disabled(dev_priv, intel_crtc->pipe); > > > > /* Filter ctl must be set before TV_WIN_SIZE */ > > - I915_WRITE(TV_FILTER_CTL_1, TV_AUTO_SCALE); > > + tv_filter_ctl = TV_AUTO_SCALE; > > + if (tv_conn_state->bypass_vfilter) > > + tv_filter_ctl |= TV_V_FILTER_BYPASS; > > + I915_WRITE(TV_FILTER_CTL_1, tv_filter_ctl); > > + > > xsize = tv_mode->hblank_start - tv_mode->hblank_end; > > ysize = intel_tv_mode_vdisplay(tv_mode); > > > > xpos = conn_state->tv.margins.left; > > - ypos = conn_state->tv.margins.top; > > + ypos = tv_conn_state->margins.top; > > xsize -= (conn_state->tv.margins.left + > > conn_state->tv.margins.right); > > - ysize -= (conn_state->tv.margins.top + > > - conn_state->tv.margins.bottom); > > + ysize -= (tv_conn_state->margins.top + > > + tv_conn_state->margins.bottom); > > I915_WRITE(TV_WIN_POS, (xpos<<16)|ypos); > > I915_WRITE(TV_WIN_SIZE, (xsize<<16)|ysize); > > > > @@ -1701,7 +1785,7 @@ static const struct drm_connector_funcs intel_tv_connector_funcs = { > > .destroy = intel_connector_destroy, > > .fill_modes = drm_helper_probe_single_connector_modes, > > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > > - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, > > + .atomic_duplicate_state = intel_tv_connector_duplicate_state, > > }; > > > > static int intel_tv_atomic_check(struct drm_connector *connector, > > -- > > 2.18.1 > > > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c index 75126fce655d..7099d837e31a 100644 --- a/drivers/gpu/drm/i915/intel_tv.c +++ b/drivers/gpu/drm/i915/intel_tv.c @@ -861,6 +861,44 @@ static const struct tv_mode tv_modes[] = { }, }; +struct intel_tv_connector_state { + struct drm_connector_state base; + + /* + * May need to override the user margins for + * gen3 >1024 wide source vertical centering. + */ + struct { + u16 top, bottom; + } margins; + + bool bypass_vfilter; +}; + +#define to_intel_tv_connector_state(x) container_of(x, struct intel_tv_connector_state, base) + +/** + * intel_digital_connector_duplicate_state - duplicate connector state + * @connector: digital connector + * + * Allocates and returns a copy of the connector state (both common and + * digital connector specific) for the specified connector. + * + * Returns: The newly allocated connector state, or NULL on failure. + */ +struct drm_connector_state * +intel_tv_connector_duplicate_state(struct drm_connector *connector) +{ + struct intel_tv_connector_state *state; + + state = kmemdup(connector->state, sizeof(*state), GFP_KERNEL); + if (!state) + return NULL; + + __drm_atomic_helper_connector_duplicate_state(connector, &state->base); + return &state->base; +} + static struct intel_tv *enc_to_tv(struct intel_encoder *encoder) { return container_of(encoder, struct intel_tv, base); @@ -1129,6 +1167,9 @@ intel_tv_compute_config(struct intel_encoder *encoder, struct intel_crtc_state *pipe_config, struct drm_connector_state *conn_state) { + struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); + struct intel_tv_connector_state *tv_conn_state = + to_intel_tv_connector_state(conn_state); const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode; @@ -1149,6 +1190,43 @@ intel_tv_compute_config(struct intel_encoder *encoder, pipe_config->port_clock = tv_mode->clock; intel_tv_mode_to_mode(adjusted_mode, tv_mode); + drm_mode_set_crtcinfo(adjusted_mode, 0); + + if (IS_GEN3(dev_priv) && hdisplay > 1024) { + int extra, top, bottom; + + extra = adjusted_mode->crtc_vdisplay - vdisplay; + + if (extra < 0) { + DRM_DEBUG_KMS("No vertical scaling for >1024 pixel wide modes\n"); + return false; + } + + /* Need to turn off the vertical filter and center the image */ + + /* Attempt to maintain the relative sizes of the margins */ + top = conn_state->tv.margins.top; + bottom = conn_state->tv.margins.bottom; + + if (top + bottom) + top = extra * top / (top + bottom); + else + top = extra / 2; + bottom = extra - top; + + tv_conn_state->margins.top = top; + tv_conn_state->margins.bottom = bottom; + + tv_conn_state->bypass_vfilter = true; + + if (!tv_mode->progressive) + adjusted_mode->flags |= DRM_MODE_FLAG_INTERLACE; + } else { + tv_conn_state->margins.top = conn_state->tv.margins.top; + tv_conn_state->margins.bottom = conn_state->tv.margins.bottom; + + tv_conn_state->bypass_vfilter = false; + } DRM_DEBUG_KMS("TV mode:\n"); drm_mode_debug_printmodeline(adjusted_mode); @@ -1222,8 +1300,8 @@ intel_tv_compute_config(struct intel_encoder *encoder, conn_state->tv.margins.left, conn_state->tv.margins.right); intel_tv_scale_mode_vert(adjusted_mode, vdisplay, - conn_state->tv.margins.top, - conn_state->tv.margins.bottom); + tv_conn_state->margins.top, + tv_conn_state->margins.bottom); drm_mode_set_crtcinfo(adjusted_mode, 0); adjusted_mode->name[0] = '\0'; @@ -1316,8 +1394,10 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, struct drm_i915_private *dev_priv = to_i915(encoder->base.dev); struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc); struct intel_tv *intel_tv = enc_to_tv(encoder); + const struct intel_tv_connector_state *tv_conn_state = + to_intel_tv_connector_state(conn_state); const struct tv_mode *tv_mode = intel_tv_mode_find(conn_state); - u32 tv_ctl; + u32 tv_ctl, tv_filter_ctl; u32 scctl1, scctl2, scctl3; int i, j; const struct video_levels *video_levels; @@ -1425,16 +1505,20 @@ static void intel_tv_pre_enable(struct intel_encoder *encoder, assert_pipe_disabled(dev_priv, intel_crtc->pipe); /* Filter ctl must be set before TV_WIN_SIZE */ - I915_WRITE(TV_FILTER_CTL_1, TV_AUTO_SCALE); + tv_filter_ctl = TV_AUTO_SCALE; + if (tv_conn_state->bypass_vfilter) + tv_filter_ctl |= TV_V_FILTER_BYPASS; + I915_WRITE(TV_FILTER_CTL_1, tv_filter_ctl); + xsize = tv_mode->hblank_start - tv_mode->hblank_end; ysize = intel_tv_mode_vdisplay(tv_mode); xpos = conn_state->tv.margins.left; - ypos = conn_state->tv.margins.top; + ypos = tv_conn_state->margins.top; xsize -= (conn_state->tv.margins.left + conn_state->tv.margins.right); - ysize -= (conn_state->tv.margins.top + - conn_state->tv.margins.bottom); + ysize -= (tv_conn_state->margins.top + + tv_conn_state->margins.bottom); I915_WRITE(TV_WIN_POS, (xpos<<16)|ypos); I915_WRITE(TV_WIN_SIZE, (xsize<<16)|ysize); @@ -1701,7 +1785,7 @@ static const struct drm_connector_funcs intel_tv_connector_funcs = { .destroy = intel_connector_destroy, .fill_modes = drm_helper_probe_single_connector_modes, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, - .atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state, + .atomic_duplicate_state = intel_tv_connector_duplicate_state, }; static int intel_tv_atomic_check(struct drm_connector *connector,