On Wed, Aug 19, 2015 at 09:11:11AM +0000, Zhang, Xiong Y wrote: > > On Fri, Aug 14, 2015 at 07:28:44PM +0300, Ville Syrjälä wrote: > > > On Fri, Aug 14, 2015 at 05:12:57AM +0000, Zhang, Xiong Y wrote: > > > > > On Mon, Aug 10, 2015 at 03:26:09PM +0800, Xiong Zhang wrote: > > > > > > Only internal eDP, LVDS, DVI screen could set scalling mode, some > > > > > > customers need to set scalling mode for external DP, HDMI, VGA screen. > > > > > > Let's fulfill this. > > > > > > > > > > > > bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=90989 > > > > > > > > > > > > Signed-off-by: Xiong Zhang <xiong.y.zhang@xxxxxxxxx> > > > > > > --- > > > > > > drivers/gpu/drm/i915/intel_dp.c | 63 > > > > > > ++++++++++++++++++++++++++++------------- > > > > > > 1 file changed, 44 insertions(+), 19 deletions(-) > > > > > > > > > > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c > > > > > > b/drivers/gpu/drm/i915/intel_dp.c index f1b9f93..2da334b 100644 > > > > > > --- a/drivers/gpu/drm/i915/intel_dp.c > > > > > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > > > > > @@ -207,7 +207,13 @@ intel_dp_mode_valid(struct drm_connector > > > > > *connector, > > > > > > int target_clock = mode->clock; > > > > > > int max_rate, mode_rate, max_lanes, max_link_clock; > > > > > > > > > > > > - if (is_edp(intel_dp) && fixed_mode) { > > > > > > + if (mode->clock < 10000) > > > > > > + return MODE_CLOCK_LOW; > > > > > > + > > > > > > + if (mode->flags & DRM_MODE_FLAG_DBLCLK) > > > > > > + return MODE_H_ILLEGAL; > > > > > > + > > > > > > + if (!intel_panel_scale_none(&intel_connector->panel)) { > > > > > > if (mode->hdisplay > fixed_mode->hdisplay) > > > > > > return MODE_PANEL; > > > > > > > > > > > > @@ -226,12 +232,6 @@ intel_dp_mode_valid(struct drm_connector > > > > > *connector, > > > > > > if (mode_rate > max_rate) > > > > > > return MODE_CLOCK_HIGH; > > > > > > > > > > > > - if (mode->clock < 10000) > > > > > > - return MODE_CLOCK_LOW; > > > > > > - > > > > > > - if (mode->flags & DRM_MODE_FLAG_DBLCLK) > > > > > > - return MODE_H_ILLEGAL; > > > > > > - > > > > > > return MODE_OK; > > > > > > } > > > > > > > > > > > > @@ -1378,7 +1378,7 @@ intel_dp_compute_config(struct > > intel_encoder > > > > > *encoder, > > > > > > pipe_config->has_drrs = false; > > > > > > pipe_config->has_audio = intel_dp->has_audio && port != PORT_A; > > > > > > > > > > > > - if (is_edp(intel_dp) && intel_connector->panel.fixed_mode) { > > > > > > + if (!intel_panel_scale_none(&intel_connector->panel)) { > > > > > > intel_fixed_panel_mode(intel_connector->panel.fixed_mode, > > > > > > adjusted_mode); > > > > > > > > > > > > @@ -4592,6 +4592,23 @@ static int intel_dp_get_modes(struct > > > > > drm_connector *connector) > > > > > > edid = intel_connector->detect_edid; > > > > > > if (edid) { > > > > > > int ret = intel_connector_update_modes(connector, edid); > > > > > > + > > > > > > + if (ret && intel_connector->panel.fixed_mode == NULL) { > > > > > > + /* init fixed mode as preferred mode for DP */ > > > > > > + struct drm_display_mode *fixed_mode = NULL; > > > > > > + struct drm_display_mode *scan; > > > > > > + > > > > > > + list_for_each_entry(scan, &connector->probed_modes, > > head) { > > > > > > + if (scan->type & DRM_MODE_TYPE_PREFERRED) > > > > > > + fixed_mode = > > drm_mode_duplicate(connector->dev, > > > > > > + scan); > > > > > > + } > > > > > > + > > > > > > + if (fixed_mode) > > > > > > + intel_panel_init(&intel_connector->panel, > > > > > > + fixed_mode, NULL); > > > > > > + } > > > > > > > > > > How are we supposed to get rid of a stale fixed_mode when some other > > > > > display gets plugged in? > > > > [Zhang, Xiong Y] Thanks so much for your good question. Yes, we should > > clear the > > > > stale fitting_mode and fixed_mode when display is disconnect in > > > > intel_dp_hpd_pulse() function. > > > > > > > > > > Also what would happen if the preferred mode can't be supported due to > > some > > > > > source limitation? > > > > [Zhang, Xiong Y] In this case, which mode should be selected as > > fixed_mode ? > > > > > > At the very least we should make sure it's a mode we can use. > > > > > > > As you said maybe kernel isn't the right place to do such decision. > > > > > > There are a lot of options how we could pick the mode. Eg. might want to > > > pick the next largest mode, and if there is none try to pick the largest > > > smaller mode (since pfit can't downscale by much). Also should we try to > > > pick an intelaced mode if the user requested one etc. Lots of open > > > questions how this policy should be handled. Would be easier to punt it > > > all to userspace, which would also avoid the kernel policy doing the > > > wrong thing when userspace knows what it wants. > > > > > > > > > > > > > In general I'm not entirely happy with having this kind of policy in the > > kernel. > > > > > I'd much prefer if we could get crtc size and border properties done so > > that > > > > > userspace could set up the scaling any which way it chooses. > > > > [Zhang, Xiong Y] Could you give more detail about your preference ? > > > > > > The idea would be to expose some sort of crtc size properties that would > > > provide pipe_src_{w,h}, and the mode would provide the actual display > > > timings as it does today. And if we add some kind of border properties > > > to control the output size of the pfit (since the mode doesn't have that > > > information) userspace could do whatever it pleased. > > > > > > I think the last attempt just sort of died out during review: > > > http://lists.freedesktop.org/archives/intel-gfx/2014-August/050732.html > > > > It occurs to me that we might actually go about this in a way that's > > more like what you did. That is, we could expose the fixed mode as a > > separate property from the user mode. That way the user could choose any > > mode as the fixed mode, keeping the policy out from the kernel, and it > > would be more in line with that we already do on LVDS/eDP. > > we'd still need to add new border properties to give userspace full > > freedom in setting up the pfit. > [Zhang, Xiong Y] Does border properties mean Center, Aspect ratio or Full ? No, the border properties would be something like "border left/right/top/bottom" in pixels. So userspace could explicitly control the position and size of the pfit output on the screen. > This sounds an available solution and let the user do the policy. > In this way, user will set two properties (fixed mode and border) to enable > Panel filter. It's a little inconvenient. > I'm afraid common user doesn't know how to choose the fixed mode and > What mean for fixed mode, Common user only know the > "Center" "Tile" like in windows. IMO that's a problem for userspace tools to solve. The kernel just provides the mecanism, and userspace can do with it what it wants. > > > > Another thing we could do with this approach is expose the pipe PF-ID > > mode when the fixed mode is interlaced and the user mode is progressive. > > And if both are interlaced, or there's just an interlaced used mode w/o > > a fixed mode, we'd keep using IF-ID like we do today. > [Zhang, Xiong Y] I checked the B spec, there isn't PF-ID / IF-ID control bit in > PIPE_CONF since BDW. I don't see any restriction on it? Are you looking at the right register (TRANSCONF bits 22:21)? I only see some SKL+ Y-tiling and 90/270 rotation restrictions. > I never see a monitor supporting interlaced mode. Does a monitor could support > both Interlaced and progressive mode ? I would expect practically every TV to have a few of them. With computer monitors I guess they don't bother anymore. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
