Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
Hi Ville, So to confirm I am skipping this patch completely. So basically keeping hw.mode as well Manasi On Tue, Sep 22, 2020 at 11:52:09AM -0700, Navare, Manasi wrote: > On Tue, Sep 22, 2020 at 01:19:15PM +0300, Ville Syrjälä wrote: > > On Mon, Sep 21, 2020 at 02:01:25PM -0700, Navare, Manasi wrote: > > > On Mon, Sep 14, 2020 at 09:52:57PM +0300, Ville Syrjälä wrote: > > > > On Mon, Sep 14, 2020 at 11:32:48AM -0700, Navare, Manasi wrote: > > > > > On Mon, Sep 07, 2020 at 03:35:23PM +0300, Ville Syrjälä wrote: > > > > > > On Thu, Sep 03, 2020 at 09:40:44PM +0300, Ville Syrjälä wrote: > > > > > > > On Thu, Sep 03, 2020 at 11:04:33AM -0700, Navare, Manasi wrote: > > > > > > > > On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote: > > > > > > > > > On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote: > > > > > > > > > > From: Maarten Lankhorst > > > > > > > > > > > > > > > > > > > > The members in hw.mode can be used from adjusted_mode as > > > > > > > > > > well, > > > > > > > > > > use that when available. > > > > > > > > > > > > > > > > > > > > Some places that use hw.mode can be converted to use > > > > > > > > > > adjusted_mode > > > > > > > > > > as well. > > > > > > > > > > > > > > > > > > > > v2: > > > > > > > > > > * Manual rebase (Manasi) > > > > > > > > > > * remove the use of pipe_mode defined in patch 3 (Manasi) > > > > > > > > > > > > > > > > > > > > v3: > > > > > > > > > > * Rebase on drm-tip (Manasi) > > > > > > > > > > > > > > > > > > Previous review was apparently ignored. Or is there a better > > > > > > > > > version > > > > > > > > > somewhere? If not, this still looks very wrong. > > > > > > > > > > > > > > > > This was the latest rev that Maarten had in his local tree > > > > > > > > which he said should address all the review comments. > > > > > > > > What in particular looks wrong or what review comments were > > > > > > > > unaddressed here? > > > > > > > > > > > > > > The dvo/sdvo changes. > > > > > > > > > > > > I recommend just dropping this patch entirely. It doesn't seem to > > > > > > have > > > > > > anything to do with the bigjoiner anyway. > > > > > > > > > > So for the dvo/svdo changes, no need to use the adjusted_mode instead > > > > > keep using hw.mode? > > > > > How about other cleanups like: > > > > > intel_crtc_copy_hw_to_uapi_state(crtc_state, ); and > > > > > static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state > > > > > *crtc_state, > > > > > + struct drm_display_mode > > > > > *user_mode) > > > > > > > > > > You think we dont need mode as an argument there either? > > > > > > > > Not in this patch if all the other stuff disappears. No idea if some > > > > later patch might need something like it. > > > > > > Hi Ville, > > > > > > So this patch basically removes the hw.mode and just keeps > > > hw.adjusted_mode > > > So no need to remove that? > > > But basically from this patch onwards we say that there is hw.pipe_mode > > > and hw.adjusted_mode, there is no hw.mode. > > > Are you suggesting keeping hw.mode as well? Would this be replacing > > > hw.pipe_mode then? > > > > No. hw.mode is the original timings, adjusted_mode is the output timings, > > pipe_mode is the the pipe timings. > > So is the suggestion to keep hw.mode so the original timings as well as > adjusted_mode and > then have pipe_mode for per pipe timings. > So get rid of this patch meaning do not remove hw.mode? > > Manasi > > > > > -- > > Ville Syrjälä > > Intel > ___ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Tue, Sep 22, 2020 at 01:19:15PM +0300, Ville Syrjälä wrote: > On Mon, Sep 21, 2020 at 02:01:25PM -0700, Navare, Manasi wrote: > > On Mon, Sep 14, 2020 at 09:52:57PM +0300, Ville Syrjälä wrote: > > > On Mon, Sep 14, 2020 at 11:32:48AM -0700, Navare, Manasi wrote: > > > > On Mon, Sep 07, 2020 at 03:35:23PM +0300, Ville Syrjälä wrote: > > > > > On Thu, Sep 03, 2020 at 09:40:44PM +0300, Ville Syrjälä wrote: > > > > > > On Thu, Sep 03, 2020 at 11:04:33AM -0700, Navare, Manasi wrote: > > > > > > > On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote: > > > > > > > > On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote: > > > > > > > > > From: Maarten Lankhorst > > > > > > > > > > > > > > > > > > The members in hw.mode can be used from adjusted_mode as well, > > > > > > > > > use that when available. > > > > > > > > > > > > > > > > > > Some places that use hw.mode can be converted to use > > > > > > > > > adjusted_mode > > > > > > > > > as well. > > > > > > > > > > > > > > > > > > v2: > > > > > > > > > * Manual rebase (Manasi) > > > > > > > > > * remove the use of pipe_mode defined in patch 3 (Manasi) > > > > > > > > > > > > > > > > > > v3: > > > > > > > > > * Rebase on drm-tip (Manasi) > > > > > > > > > > > > > > > > Previous review was apparently ignored. Or is there a better > > > > > > > > version > > > > > > > > somewhere? If not, this still looks very wrong. > > > > > > > > > > > > > > This was the latest rev that Maarten had in his local tree which > > > > > > > he said should address all the review comments. > > > > > > > What in particular looks wrong or what review comments were > > > > > > > unaddressed here? > > > > > > > > > > > > The dvo/sdvo changes. > > > > > > > > > > I recommend just dropping this patch entirely. It doesn't seem to have > > > > > anything to do with the bigjoiner anyway. > > > > > > > > So for the dvo/svdo changes, no need to use the adjusted_mode instead > > > > keep using hw.mode? > > > > How about other cleanups like: > > > > intel_crtc_copy_hw_to_uapi_state(crtc_state, ); and > > > > static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state > > > > *crtc_state, > > > > +struct drm_display_mode > > > > *user_mode) > > > > > > > > You think we dont need mode as an argument there either? > > > > > > Not in this patch if all the other stuff disappears. No idea if some > > > later patch might need something like it. > > > > Hi Ville, > > > > So this patch basically removes the hw.mode and just keeps hw.adjusted_mode > > So no need to remove that? > > But basically from this patch onwards we say that there is hw.pipe_mode > > and hw.adjusted_mode, there is no hw.mode. > > Are you suggesting keeping hw.mode as well? Would this be replacing > > hw.pipe_mode then? > > No. hw.mode is the original timings, adjusted_mode is the output timings, > pipe_mode is the the pipe timings. So is the suggestion to keep hw.mode so the original timings as well as adjusted_mode and then have pipe_mode for per pipe timings. So get rid of this patch meaning do not remove hw.mode? Manasi > > -- > Ville Syrjälä > Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Mon, Sep 21, 2020 at 02:01:25PM -0700, Navare, Manasi wrote: > On Mon, Sep 14, 2020 at 09:52:57PM +0300, Ville Syrjälä wrote: > > On Mon, Sep 14, 2020 at 11:32:48AM -0700, Navare, Manasi wrote: > > > On Mon, Sep 07, 2020 at 03:35:23PM +0300, Ville Syrjälä wrote: > > > > On Thu, Sep 03, 2020 at 09:40:44PM +0300, Ville Syrjälä wrote: > > > > > On Thu, Sep 03, 2020 at 11:04:33AM -0700, Navare, Manasi wrote: > > > > > > On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote: > > > > > > > On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote: > > > > > > > > From: Maarten Lankhorst > > > > > > > > > > > > > > > > The members in hw.mode can be used from adjusted_mode as well, > > > > > > > > use that when available. > > > > > > > > > > > > > > > > Some places that use hw.mode can be converted to use > > > > > > > > adjusted_mode > > > > > > > > as well. > > > > > > > > > > > > > > > > v2: > > > > > > > > * Manual rebase (Manasi) > > > > > > > > * remove the use of pipe_mode defined in patch 3 (Manasi) > > > > > > > > > > > > > > > > v3: > > > > > > > > * Rebase on drm-tip (Manasi) > > > > > > > > > > > > > > Previous review was apparently ignored. Or is there a better > > > > > > > version > > > > > > > somewhere? If not, this still looks very wrong. > > > > > > > > > > > > This was the latest rev that Maarten had in his local tree which he > > > > > > said should address all the review comments. > > > > > > What in particular looks wrong or what review comments were > > > > > > unaddressed here? > > > > > > > > > > The dvo/sdvo changes. > > > > > > > > I recommend just dropping this patch entirely. It doesn't seem to have > > > > anything to do with the bigjoiner anyway. > > > > > > So for the dvo/svdo changes, no need to use the adjusted_mode instead > > > keep using hw.mode? > > > How about other cleanups like: > > > intel_crtc_copy_hw_to_uapi_state(crtc_state, ); and > > > static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state > > > *crtc_state, > > > + struct drm_display_mode *user_mode) > > > > > > You think we dont need mode as an argument there either? > > > > Not in this patch if all the other stuff disappears. No idea if some > > later patch might need something like it. > > Hi Ville, > > So this patch basically removes the hw.mode and just keeps hw.adjusted_mode > So no need to remove that? > But basically from this patch onwards we say that there is hw.pipe_mode > and hw.adjusted_mode, there is no hw.mode. > Are you suggesting keeping hw.mode as well? Would this be replacing > hw.pipe_mode then? No. hw.mode is the original timings, adjusted_mode is the output timings, pipe_mode is the the pipe timings. -- Ville Syrjälä Intel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Mon, Sep 14, 2020 at 09:52:57PM +0300, Ville Syrjälä wrote:
> On Mon, Sep 14, 2020 at 11:32:48AM -0700, Navare, Manasi wrote:
> > On Mon, Sep 07, 2020 at 03:35:23PM +0300, Ville Syrjälä wrote:
> > > On Thu, Sep 03, 2020 at 09:40:44PM +0300, Ville Syrjälä wrote:
> > > > On Thu, Sep 03, 2020 at 11:04:33AM -0700, Navare, Manasi wrote:
> > > > > On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote:
> > > > > > On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote:
> > > > > > > From: Maarten Lankhorst
> > > > > > >
> > > > > > > The members in hw.mode can be used from adjusted_mode as well,
> > > > > > > use that when available.
> > > > > > >
> > > > > > > Some places that use hw.mode can be converted to use adjusted_mode
> > > > > > > as well.
> > > > > > >
> > > > > > > v2:
> > > > > > > * Manual rebase (Manasi)
> > > > > > > * remove the use of pipe_mode defined in patch 3 (Manasi)
> > > > > > >
> > > > > > > v3:
> > > > > > > * Rebase on drm-tip (Manasi)
> > > > > >
> > > > > > Previous review was apparently ignored. Or is there a better version
> > > > > > somewhere? If not, this still looks very wrong.
> > > > >
> > > > > This was the latest rev that Maarten had in his local tree which he
> > > > > said should address all the review comments.
> > > > > What in particular looks wrong or what review comments were
> > > > > unaddressed here?
> > > >
> > > > The dvo/sdvo changes.
> > >
> > > I recommend just dropping this patch entirely. It doesn't seem to have
> > > anything to do with the bigjoiner anyway.
> >
> > So for the dvo/svdo changes, no need to use the adjusted_mode instead keep
> > using hw.mode?
> > How about other cleanups like: intel_crtc_copy_hw_to_uapi_state(crtc_state,
> > ); and
> > static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > *crtc_state,
> > +struct drm_display_mode *user_mode)
> >
> > You think we dont need mode as an argument there either?
>
> Not in this patch if all the other stuff disappears. No idea if some
> later patch might need something like it.
Hi Ville,
So this patch basically removes the hw.mode and just keeps hw.adjusted_mode
So no need to remove that?
But basically from this patch onwards we say that there is hw.pipe_mode
and hw.adjusted_mode, there is no hw.mode.
Are you suggesting keeping hw.mode as well? Would this be replacing
hw.pipe_mode then?
Manasi
>
> >
> > Manasi
> > >
> > > >
> > > > >
> > > > > @Maarten any feedback on Ville's unaddressed comments?
> > > > >
> > > > > Manasi
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Signed-off-by: Maarten Lankhorst
> > > > > > >
> > > > > > > Signed-off-by: Manasi Navare
> > > > > > > ---
> > > > > > > drivers/gpu/drm/i915/display/intel_display.c | 29
> > > > > > > ++-
> > > > > > > .../drm/i915/display/intel_display_types.h| 2 +-
> > > > > > > drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
> > > > > > > drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
> > > > > > > 4 files changed, 23 insertions(+), 26 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > index 729ec6e0d43a..8652a7c6bf11 100644
> > > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > > @@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct
> > > > > > > intel_crtc *crtc,
> > > > > > > tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
> > > > > > > pipe_config->pipe_src_h = (tmp & 0x) + 1;
> > > > > > > pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
> > > > > > > -
> > > > > > > - pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
> > > > > > > - pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
> > > > > > > }
> > > > > > >
> > > > > > > void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> > > > > > > @@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const
> > > > > > > struct intel_crtc_state *pipe_config,
> > > > > > > intel_dump_dp_vsc_sdp(dev_priv,
> > > > > > > _config->infoframes.vsc);
> > > > > > >
> > > > > > > drm_dbg_kms(_priv->drm, "requested mode:\n");
> > > > > > > - drm_mode_debug_printmodeline(_config->hw.mode);
> > > > > > > + drm_mode_debug_printmodeline(_config->uapi.mode);
> > > > > > > drm_dbg_kms(_priv->drm, "adjusted mode:\n");
> > > > > > > drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
> > > > > > > intel_dump_crtc_timings(dev_priv,
> > > > > > > _config->hw.adjusted_mode);
> > > > > > > @@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
> > > > > > > intel_crtc_state *crtc_state)
> > > > > > > {
> > > > > > > crtc_state->hw.enable = crtc_state->uapi.enable;
> > > > > > > crtc_state->hw.active = crtc_state->uapi.active;
> > > > > > > -
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Mon, Sep 14, 2020 at 11:32:48AM -0700, Navare, Manasi wrote:
> On Mon, Sep 07, 2020 at 03:35:23PM +0300, Ville Syrjälä wrote:
> > On Thu, Sep 03, 2020 at 09:40:44PM +0300, Ville Syrjälä wrote:
> > > On Thu, Sep 03, 2020 at 11:04:33AM -0700, Navare, Manasi wrote:
> > > > On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote:
> > > > > On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote:
> > > > > > From: Maarten Lankhorst
> > > > > >
> > > > > > The members in hw.mode can be used from adjusted_mode as well,
> > > > > > use that when available.
> > > > > >
> > > > > > Some places that use hw.mode can be converted to use adjusted_mode
> > > > > > as well.
> > > > > >
> > > > > > v2:
> > > > > > * Manual rebase (Manasi)
> > > > > > * remove the use of pipe_mode defined in patch 3 (Manasi)
> > > > > >
> > > > > > v3:
> > > > > > * Rebase on drm-tip (Manasi)
> > > > >
> > > > > Previous review was apparently ignored. Or is there a better version
> > > > > somewhere? If not, this still looks very wrong.
> > > >
> > > > This was the latest rev that Maarten had in his local tree which he
> > > > said should address all the review comments.
> > > > What in particular looks wrong or what review comments were unaddressed
> > > > here?
> > >
> > > The dvo/sdvo changes.
> >
> > I recommend just dropping this patch entirely. It doesn't seem to have
> > anything to do with the bigjoiner anyway.
>
> So for the dvo/svdo changes, no need to use the adjusted_mode instead keep
> using hw.mode?
> How about other cleanups like: intel_crtc_copy_hw_to_uapi_state(crtc_state,
> ); and
> static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> *crtc_state,
> + struct drm_display_mode *user_mode)
>
> You think we dont need mode as an argument there either?
Not in this patch if all the other stuff disappears. No idea if some
later patch might need something like it.
>
> Manasi
> >
> > >
> > > >
> > > > @Maarten any feedback on Ville's unaddressed comments?
> > > >
> > > > Manasi
> > > >
> > > > >
> > > > > >
> > > > > > Signed-off-by: Maarten Lankhorst
> > > > > > Signed-off-by: Manasi Navare
> > > > > > ---
> > > > > > drivers/gpu/drm/i915/display/intel_display.c | 29
> > > > > > ++-
> > > > > > .../drm/i915/display/intel_display_types.h| 2 +-
> > > > > > drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
> > > > > > drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
> > > > > > 4 files changed, 23 insertions(+), 26 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > index 729ec6e0d43a..8652a7c6bf11 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > > @@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct
> > > > > > intel_crtc *crtc,
> > > > > > tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
> > > > > > pipe_config->pipe_src_h = (tmp & 0x) + 1;
> > > > > > pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
> > > > > > -
> > > > > > - pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
> > > > > > - pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
> > > > > > }
> > > > > >
> > > > > > void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> > > > > > @@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const
> > > > > > struct intel_crtc_state *pipe_config,
> > > > > > intel_dump_dp_vsc_sdp(dev_priv,
> > > > > > _config->infoframes.vsc);
> > > > > >
> > > > > > drm_dbg_kms(_priv->drm, "requested mode:\n");
> > > > > > - drm_mode_debug_printmodeline(_config->hw.mode);
> > > > > > + drm_mode_debug_printmodeline(_config->uapi.mode);
> > > > > > drm_dbg_kms(_priv->drm, "adjusted mode:\n");
> > > > > > drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
> > > > > > intel_dump_crtc_timings(dev_priv,
> > > > > > _config->hw.adjusted_mode);
> > > > > > @@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
> > > > > > intel_crtc_state *crtc_state)
> > > > > > {
> > > > > > crtc_state->hw.enable = crtc_state->uapi.enable;
> > > > > > crtc_state->hw.active = crtc_state->uapi.active;
> > > > > > - crtc_state->hw.mode = crtc_state->uapi.mode;
> > > > > > crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
> > > > > > intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
> > > > > > }
> > > > > >
> > > > > > -static void intel_crtc_copy_hw_to_uapi_state(struct
> > > > > > intel_crtc_state *crtc_state)
> > > > > > +static void intel_crtc_copy_hw_to_uapi_state(struct
> > > > > > intel_crtc_state *crtc_state,
> > > > > > +struct drm_display_mode
> > > > > > *user_mode)
> > > > > > {
> > > > > >
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Mon, Sep 07, 2020 at 03:35:23PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 03, 2020 at 09:40:44PM +0300, Ville Syrjälä wrote:
> > On Thu, Sep 03, 2020 at 11:04:33AM -0700, Navare, Manasi wrote:
> > > On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote:
> > > > On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote:
> > > > > From: Maarten Lankhorst
> > > > >
> > > > > The members in hw.mode can be used from adjusted_mode as well,
> > > > > use that when available.
> > > > >
> > > > > Some places that use hw.mode can be converted to use adjusted_mode
> > > > > as well.
> > > > >
> > > > > v2:
> > > > > * Manual rebase (Manasi)
> > > > > * remove the use of pipe_mode defined in patch 3 (Manasi)
> > > > >
> > > > > v3:
> > > > > * Rebase on drm-tip (Manasi)
> > > >
> > > > Previous review was apparently ignored. Or is there a better version
> > > > somewhere? If not, this still looks very wrong.
> > >
> > > This was the latest rev that Maarten had in his local tree which he said
> > > should address all the review comments.
> > > What in particular looks wrong or what review comments were unaddressed
> > > here?
> >
> > The dvo/sdvo changes.
>
> I recommend just dropping this patch entirely. It doesn't seem to have
> anything to do with the bigjoiner anyway.
So for the dvo/svdo changes, no need to use the adjusted_mode instead keep
using hw.mode?
How about other cleanups like: intel_crtc_copy_hw_to_uapi_state(crtc_state,
); and
static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
*crtc_state,
+struct drm_display_mode *user_mode)
You think we dont need mode as an argument there either?
Manasi
>
> >
> > >
> > > @Maarten any feedback on Ville's unaddressed comments?
> > >
> > > Manasi
> > >
> > > >
> > > > >
> > > > > Signed-off-by: Maarten Lankhorst
> > > > > Signed-off-by: Manasi Navare
> > > > > ---
> > > > > drivers/gpu/drm/i915/display/intel_display.c | 29
> > > > > ++-
> > > > > .../drm/i915/display/intel_display_types.h| 2 +-
> > > > > drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
> > > > > drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
> > > > > 4 files changed, 23 insertions(+), 26 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > index 729ec6e0d43a..8652a7c6bf11 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > > @@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct
> > > > > intel_crtc *crtc,
> > > > > tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
> > > > > pipe_config->pipe_src_h = (tmp & 0x) + 1;
> > > > > pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
> > > > > -
> > > > > - pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
> > > > > - pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
> > > > > }
> > > > >
> > > > > void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> > > > > @@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const
> > > > > struct intel_crtc_state *pipe_config,
> > > > > intel_dump_dp_vsc_sdp(dev_priv,
> > > > > _config->infoframes.vsc);
> > > > >
> > > > > drm_dbg_kms(_priv->drm, "requested mode:\n");
> > > > > - drm_mode_debug_printmodeline(_config->hw.mode);
> > > > > + drm_mode_debug_printmodeline(_config->uapi.mode);
> > > > > drm_dbg_kms(_priv->drm, "adjusted mode:\n");
> > > > > drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
> > > > > intel_dump_crtc_timings(dev_priv,
> > > > > _config->hw.adjusted_mode);
> > > > > @@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
> > > > > intel_crtc_state *crtc_state)
> > > > > {
> > > > > crtc_state->hw.enable = crtc_state->uapi.enable;
> > > > > crtc_state->hw.active = crtc_state->uapi.active;
> > > > > - crtc_state->hw.mode = crtc_state->uapi.mode;
> > > > > crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
> > > > > intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
> > > > > }
> > > > >
> > > > > -static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > > > > *crtc_state)
> > > > > +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > > > > *crtc_state,
> > > > > + struct drm_display_mode
> > > > > *user_mode)
> > > > > {
> > > > > crtc_state->uapi.enable = crtc_state->hw.enable;
> > > > > crtc_state->uapi.active = crtc_state->hw.active;
> > > > > drm_WARN_ON(crtc_state->uapi.crtc->dev,
> > > > > - drm_atomic_set_mode_for_crtc(_state->uapi,
> > > > > _state->hw.mode) < 0);
> > > > > + drm_atomic_set_mode_for_crtc(_state->uapi,
> > > > >
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Thu, Sep 03, 2020 at 09:40:44PM +0300, Ville Syrjälä wrote:
> On Thu, Sep 03, 2020 at 11:04:33AM -0700, Navare, Manasi wrote:
> > On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote:
> > > On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote:
> > > > From: Maarten Lankhorst
> > > >
> > > > The members in hw.mode can be used from adjusted_mode as well,
> > > > use that when available.
> > > >
> > > > Some places that use hw.mode can be converted to use adjusted_mode
> > > > as well.
> > > >
> > > > v2:
> > > > * Manual rebase (Manasi)
> > > > * remove the use of pipe_mode defined in patch 3 (Manasi)
> > > >
> > > > v3:
> > > > * Rebase on drm-tip (Manasi)
> > >
> > > Previous review was apparently ignored. Or is there a better version
> > > somewhere? If not, this still looks very wrong.
> >
> > This was the latest rev that Maarten had in his local tree which he said
> > should address all the review comments.
> > What in particular looks wrong or what review comments were unaddressed
> > here?
>
> The dvo/sdvo changes.
I recommend just dropping this patch entirely. It doesn't seem to have
anything to do with the bigjoiner anyway.
>
> >
> > @Maarten any feedback on Ville's unaddressed comments?
> >
> > Manasi
> >
> > >
> > > >
> > > > Signed-off-by: Maarten Lankhorst
> > > > Signed-off-by: Manasi Navare
> > > > ---
> > > > drivers/gpu/drm/i915/display/intel_display.c | 29 ++-
> > > > .../drm/i915/display/intel_display_types.h| 2 +-
> > > > drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
> > > > drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
> > > > 4 files changed, 23 insertions(+), 26 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > > index 729ec6e0d43a..8652a7c6bf11 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > > @@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct
> > > > intel_crtc *crtc,
> > > > tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
> > > > pipe_config->pipe_src_h = (tmp & 0x) + 1;
> > > > pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
> > > > -
> > > > - pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
> > > > - pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
> > > > }
> > > >
> > > > void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> > > > @@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const struct
> > > > intel_crtc_state *pipe_config,
> > > > intel_dump_dp_vsc_sdp(dev_priv,
> > > > _config->infoframes.vsc);
> > > >
> > > > drm_dbg_kms(_priv->drm, "requested mode:\n");
> > > > - drm_mode_debug_printmodeline(_config->hw.mode);
> > > > + drm_mode_debug_printmodeline(_config->uapi.mode);
> > > > drm_dbg_kms(_priv->drm, "adjusted mode:\n");
> > > > drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
> > > > intel_dump_crtc_timings(dev_priv,
> > > > _config->hw.adjusted_mode);
> > > > @@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
> > > > intel_crtc_state *crtc_state)
> > > > {
> > > > crtc_state->hw.enable = crtc_state->uapi.enable;
> > > > crtc_state->hw.active = crtc_state->uapi.active;
> > > > - crtc_state->hw.mode = crtc_state->uapi.mode;
> > > > crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
> > > > intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
> > > > }
> > > >
> > > > -static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > > > *crtc_state)
> > > > +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > > > *crtc_state,
> > > > +struct drm_display_mode
> > > > *user_mode)
> > > > {
> > > > crtc_state->uapi.enable = crtc_state->hw.enable;
> > > > crtc_state->uapi.active = crtc_state->hw.active;
> > > > drm_WARN_ON(crtc_state->uapi.crtc->dev,
> > > > - drm_atomic_set_mode_for_crtc(_state->uapi,
> > > > _state->hw.mode) < 0);
> > > > + drm_atomic_set_mode_for_crtc(_state->uapi,
> > > > user_mode) < 0);
> > > >
> > > > crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
> > > >
> > > > @@ -13277,6 +13274,10 @@ intel_crtc_prepare_cleared_state(struct
> > > > intel_crtc_state *crtc_state)
> > > > memcpy(crtc_state, saved_state, sizeof(*crtc_state));
> > > > kfree(saved_state);
> > > >
> > > > + /* Clear I915_MODE_FLAG_INHERITED */
> > > > + crtc_state->uapi.mode.private_flags = 0;
> > > > + crtc_state->uapi.adjusted_mode.private_flags = 0;
> > > > +
> > > > intel_crtc_copy_uapi_to_hw_state(crtc_state);
> > > >
> > > > return 0;
> > > >
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Thu, Sep 03, 2020 at 11:04:33AM -0700, Navare, Manasi wrote:
> On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote:
> > On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote:
> > > From: Maarten Lankhorst
> > >
> > > The members in hw.mode can be used from adjusted_mode as well,
> > > use that when available.
> > >
> > > Some places that use hw.mode can be converted to use adjusted_mode
> > > as well.
> > >
> > > v2:
> > > * Manual rebase (Manasi)
> > > * remove the use of pipe_mode defined in patch 3 (Manasi)
> > >
> > > v3:
> > > * Rebase on drm-tip (Manasi)
> >
> > Previous review was apparently ignored. Or is there a better version
> > somewhere? If not, this still looks very wrong.
>
> This was the latest rev that Maarten had in his local tree which he said
> should address all the review comments.
> What in particular looks wrong or what review comments were unaddressed here?
The dvo/sdvo changes.
>
> @Maarten any feedback on Ville's unaddressed comments?
>
> Manasi
>
> >
> > >
> > > Signed-off-by: Maarten Lankhorst
> > > Signed-off-by: Manasi Navare
> > > ---
> > > drivers/gpu/drm/i915/display/intel_display.c | 29 ++-
> > > .../drm/i915/display/intel_display_types.h| 2 +-
> > > drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
> > > drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
> > > 4 files changed, 23 insertions(+), 26 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index 729ec6e0d43a..8652a7c6bf11 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct
> > > intel_crtc *crtc,
> > > tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
> > > pipe_config->pipe_src_h = (tmp & 0x) + 1;
> > > pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
> > > -
> > > - pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
> > > - pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
> > > }
> > >
> > > void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> > > @@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const struct
> > > intel_crtc_state *pipe_config,
> > > intel_dump_dp_vsc_sdp(dev_priv, _config->infoframes.vsc);
> > >
> > > drm_dbg_kms(_priv->drm, "requested mode:\n");
> > > - drm_mode_debug_printmodeline(_config->hw.mode);
> > > + drm_mode_debug_printmodeline(_config->uapi.mode);
> > > drm_dbg_kms(_priv->drm, "adjusted mode:\n");
> > > drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
> > > intel_dump_crtc_timings(dev_priv, _config->hw.adjusted_mode);
> > > @@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
> > > intel_crtc_state *crtc_state)
> > > {
> > > crtc_state->hw.enable = crtc_state->uapi.enable;
> > > crtc_state->hw.active = crtc_state->uapi.active;
> > > - crtc_state->hw.mode = crtc_state->uapi.mode;
> > > crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
> > > intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
> > > }
> > >
> > > -static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > > *crtc_state)
> > > +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > > *crtc_state,
> > > + struct drm_display_mode *user_mode)
> > > {
> > > crtc_state->uapi.enable = crtc_state->hw.enable;
> > > crtc_state->uapi.active = crtc_state->hw.active;
> > > drm_WARN_ON(crtc_state->uapi.crtc->dev,
> > > - drm_atomic_set_mode_for_crtc(_state->uapi,
> > > _state->hw.mode) < 0);
> > > + drm_atomic_set_mode_for_crtc(_state->uapi, user_mode)
> > > < 0);
> > >
> > > crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
> > >
> > > @@ -13277,6 +13274,10 @@ intel_crtc_prepare_cleared_state(struct
> > > intel_crtc_state *crtc_state)
> > > memcpy(crtc_state, saved_state, sizeof(*crtc_state));
> > > kfree(saved_state);
> > >
> > > + /* Clear I915_MODE_FLAG_INHERITED */
> > > + crtc_state->uapi.mode.private_flags = 0;
> > > + crtc_state->uapi.adjusted_mode.private_flags = 0;
> > > +
> > > intel_crtc_copy_uapi_to_hw_state(crtc_state);
> > >
> > > return 0;
> > > @@ -13324,7 +13325,7 @@ intel_modeset_pipe_config(struct intel_crtc_state
> > > *pipe_config)
> > >* computation to clearly distinguish it from the adjusted mode, which
> > >* can be changed by the connectors in the below retry loop.
> > >*/
> > > - drm_mode_get_hv_timing(_config->hw.mode,
> > > + drm_mode_get_hv_timing(_config->hw.adjusted_mode,
> > > _config->pipe_src_w,
> > > _config->pipe_src_h);
> > >
> > > @@ -18461,15 +18462,11 @@ static void
> > > intel_modeset_readout_hw_state(struct drm_device *dev)
> > > int min_cdclk =
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Thu, Sep 03, 2020 at 08:49:44PM +0300, Ville Syrjälä wrote:
> On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote:
> > From: Maarten Lankhorst
> >
> > The members in hw.mode can be used from adjusted_mode as well,
> > use that when available.
> >
> > Some places that use hw.mode can be converted to use adjusted_mode
> > as well.
> >
> > v2:
> > * Manual rebase (Manasi)
> > * remove the use of pipe_mode defined in patch 3 (Manasi)
> >
> > v3:
> > * Rebase on drm-tip (Manasi)
>
> Previous review was apparently ignored. Or is there a better version
> somewhere? If not, this still looks very wrong.
This was the latest rev that Maarten had in his local tree which he said should
address all the review comments.
What in particular looks wrong or what review comments were unaddressed here?
@Maarten any feedback on Ville's unaddressed comments?
Manasi
>
> >
> > Signed-off-by: Maarten Lankhorst
> > Signed-off-by: Manasi Navare
> > ---
> > drivers/gpu/drm/i915/display/intel_display.c | 29 ++-
> > .../drm/i915/display/intel_display_types.h| 2 +-
> > drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
> > drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
> > 4 files changed, 23 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > b/drivers/gpu/drm/i915/display/intel_display.c
> > index 729ec6e0d43a..8652a7c6bf11 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct intel_crtc
> > *crtc,
> > tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
> > pipe_config->pipe_src_h = (tmp & 0x) + 1;
> > pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
> > -
> > - pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
> > - pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
> > }
> >
> > void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> > @@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const struct
> > intel_crtc_state *pipe_config,
> > intel_dump_dp_vsc_sdp(dev_priv, _config->infoframes.vsc);
> >
> > drm_dbg_kms(_priv->drm, "requested mode:\n");
> > - drm_mode_debug_printmodeline(_config->hw.mode);
> > + drm_mode_debug_printmodeline(_config->uapi.mode);
> > drm_dbg_kms(_priv->drm, "adjusted mode:\n");
> > drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
> > intel_dump_crtc_timings(dev_priv, _config->hw.adjusted_mode);
> > @@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
> > intel_crtc_state *crtc_state)
> > {
> > crtc_state->hw.enable = crtc_state->uapi.enable;
> > crtc_state->hw.active = crtc_state->uapi.active;
> > - crtc_state->hw.mode = crtc_state->uapi.mode;
> > crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
> > intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
> > }
> >
> > -static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > *crtc_state)
> > +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> > *crtc_state,
> > +struct drm_display_mode *user_mode)
> > {
> > crtc_state->uapi.enable = crtc_state->hw.enable;
> > crtc_state->uapi.active = crtc_state->hw.active;
> > drm_WARN_ON(crtc_state->uapi.crtc->dev,
> > - drm_atomic_set_mode_for_crtc(_state->uapi,
> > _state->hw.mode) < 0);
> > + drm_atomic_set_mode_for_crtc(_state->uapi, user_mode)
> > < 0);
> >
> > crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
> >
> > @@ -13277,6 +13274,10 @@ intel_crtc_prepare_cleared_state(struct
> > intel_crtc_state *crtc_state)
> > memcpy(crtc_state, saved_state, sizeof(*crtc_state));
> > kfree(saved_state);
> >
> > + /* Clear I915_MODE_FLAG_INHERITED */
> > + crtc_state->uapi.mode.private_flags = 0;
> > + crtc_state->uapi.adjusted_mode.private_flags = 0;
> > +
> > intel_crtc_copy_uapi_to_hw_state(crtc_state);
> >
> > return 0;
> > @@ -13324,7 +13325,7 @@ intel_modeset_pipe_config(struct intel_crtc_state
> > *pipe_config)
> > * computation to clearly distinguish it from the adjusted mode, which
> > * can be changed by the connectors in the below retry loop.
> > */
> > - drm_mode_get_hv_timing(_config->hw.mode,
> > + drm_mode_get_hv_timing(_config->hw.adjusted_mode,
> >_config->pipe_src_w,
> >_config->pipe_src_h);
> >
> > @@ -18461,15 +18462,11 @@ static void intel_modeset_readout_hw_state(struct
> > drm_device *dev)
> > int min_cdclk = 0;
> >
> > if (crtc_state->hw.active) {
> > - struct drm_display_mode *mode = _state->hw.mode;
> > + struct drm_display_mode mode;
> >
> >
> >
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On Wed, Jul 15, 2020 at 03:42:13PM -0700, Manasi Navare wrote:
> From: Maarten Lankhorst
>
> The members in hw.mode can be used from adjusted_mode as well,
> use that when available.
>
> Some places that use hw.mode can be converted to use adjusted_mode
> as well.
>
> v2:
> * Manual rebase (Manasi)
> * remove the use of pipe_mode defined in patch 3 (Manasi)
>
> v3:
> * Rebase on drm-tip (Manasi)
Previous review was apparently ignored. Or is there a better version
somewhere? If not, this still looks very wrong.
>
> Signed-off-by: Maarten Lankhorst
> Signed-off-by: Manasi Navare
> ---
> drivers/gpu/drm/i915/display/intel_display.c | 29 ++-
> .../drm/i915/display/intel_display_types.h| 2 +-
> drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
> drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
> 4 files changed, 23 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 729ec6e0d43a..8652a7c6bf11 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct intel_crtc
> *crtc,
> tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
> pipe_config->pipe_src_h = (tmp & 0x) + 1;
> pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
> -
> - pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
> - pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
> }
>
> void intel_mode_from_pipe_config(struct drm_display_mode *mode,
> @@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const struct
> intel_crtc_state *pipe_config,
> intel_dump_dp_vsc_sdp(dev_priv, _config->infoframes.vsc);
>
> drm_dbg_kms(_priv->drm, "requested mode:\n");
> - drm_mode_debug_printmodeline(_config->hw.mode);
> + drm_mode_debug_printmodeline(_config->uapi.mode);
> drm_dbg_kms(_priv->drm, "adjusted mode:\n");
> drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
> intel_dump_crtc_timings(dev_priv, _config->hw.adjusted_mode);
> @@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
> intel_crtc_state *crtc_state)
> {
> crtc_state->hw.enable = crtc_state->uapi.enable;
> crtc_state->hw.active = crtc_state->uapi.active;
> - crtc_state->hw.mode = crtc_state->uapi.mode;
> crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
> intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
> }
>
> -static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> *crtc_state)
> +static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
> *crtc_state,
> + struct drm_display_mode *user_mode)
> {
> crtc_state->uapi.enable = crtc_state->hw.enable;
> crtc_state->uapi.active = crtc_state->hw.active;
> drm_WARN_ON(crtc_state->uapi.crtc->dev,
> - drm_atomic_set_mode_for_crtc(_state->uapi,
> _state->hw.mode) < 0);
> + drm_atomic_set_mode_for_crtc(_state->uapi, user_mode)
> < 0);
>
> crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
>
> @@ -13277,6 +13274,10 @@ intel_crtc_prepare_cleared_state(struct
> intel_crtc_state *crtc_state)
> memcpy(crtc_state, saved_state, sizeof(*crtc_state));
> kfree(saved_state);
>
> + /* Clear I915_MODE_FLAG_INHERITED */
> + crtc_state->uapi.mode.private_flags = 0;
> + crtc_state->uapi.adjusted_mode.private_flags = 0;
> +
> intel_crtc_copy_uapi_to_hw_state(crtc_state);
>
> return 0;
> @@ -13324,7 +13325,7 @@ intel_modeset_pipe_config(struct intel_crtc_state
> *pipe_config)
>* computation to clearly distinguish it from the adjusted mode, which
>* can be changed by the connectors in the below retry loop.
>*/
> - drm_mode_get_hv_timing(_config->hw.mode,
> + drm_mode_get_hv_timing(_config->hw.adjusted_mode,
> _config->pipe_src_w,
> _config->pipe_src_h);
>
> @@ -18461,15 +18462,11 @@ static void intel_modeset_readout_hw_state(struct
> drm_device *dev)
> int min_cdclk = 0;
>
> if (crtc_state->hw.active) {
> - struct drm_display_mode *mode = _state->hw.mode;
> + struct drm_display_mode mode;
>
>
> intel_mode_from_pipe_config(_state->hw.adjusted_mode,
> crtc_state);
>
> - *mode = crtc_state->hw.adjusted_mode;
> - mode->hdisplay = crtc_state->pipe_src_w;
> - mode->vdisplay = crtc_state->pipe_src_h;
> -
> /*
>* The initial mode needs to be set in order to keep
>* the atomic core happy. It wants a
Re: [Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
On 16-07-2020 04:12, Manasi Navare wrote:
From: Maarten Lankhorst
The members in hw.mode can be used from adjusted_mode as well,
use that when available.
Some places that use hw.mode can be converted to use adjusted_mode
as well.
v2:
* Manual rebase (Manasi)
* remove the use of pipe_mode defined in patch 3 (Manasi)
v3:
* Rebase on drm-tip (Manasi)
Signed-off-by: Maarten Lankhorst
Signed-off-by: Manasi Navare
Changes looks ok to me.
Reviewed-by: Animesh Manna
---
drivers/gpu/drm/i915/display/intel_display.c | 29 ++-
.../drm/i915/display/intel_display_types.h| 2 +-
drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
4 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 729ec6e0d43a..8652a7c6bf11 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct intel_crtc
*crtc,
tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
pipe_config->pipe_src_h = (tmp & 0x) + 1;
pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
-
- pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
- pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
}
void intel_mode_from_pipe_config(struct drm_display_mode *mode,
@@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const struct
intel_crtc_state *pipe_config,
intel_dump_dp_vsc_sdp(dev_priv, _config->infoframes.vsc);
drm_dbg_kms(_priv->drm, "requested mode:\n");
- drm_mode_debug_printmodeline(_config->hw.mode);
+ drm_mode_debug_printmodeline(_config->uapi.mode);
drm_dbg_kms(_priv->drm, "adjusted mode:\n");
drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
intel_dump_crtc_timings(dev_priv, _config->hw.adjusted_mode);
@@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
intel_crtc_state *crtc_state)
{
crtc_state->hw.enable = crtc_state->uapi.enable;
crtc_state->hw.active = crtc_state->uapi.active;
- crtc_state->hw.mode = crtc_state->uapi.mode;
crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
}
-static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state)
+static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
*crtc_state,
+struct drm_display_mode *user_mode)
{
crtc_state->uapi.enable = crtc_state->hw.enable;
crtc_state->uapi.active = crtc_state->hw.active;
drm_WARN_ON(crtc_state->uapi.crtc->dev,
- drm_atomic_set_mode_for_crtc(_state->uapi,
_state->hw.mode) < 0);
+ drm_atomic_set_mode_for_crtc(_state->uapi, user_mode)
< 0);
crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
@@ -13277,6 +13274,10 @@ intel_crtc_prepare_cleared_state(struct intel_crtc_state *crtc_state)
memcpy(crtc_state, saved_state, sizeof(*crtc_state));
kfree(saved_state);
+ /* Clear I915_MODE_FLAG_INHERITED */
+ crtc_state->uapi.mode.private_flags = 0;
+ crtc_state->uapi.adjusted_mode.private_flags = 0;
+
intel_crtc_copy_uapi_to_hw_state(crtc_state);
return 0;
@@ -13324,7 +13325,7 @@ intel_modeset_pipe_config(struct intel_crtc_state
*pipe_config)
* computation to clearly distinguish it from the adjusted mode, which
* can be changed by the connectors in the below retry loop.
*/
- drm_mode_get_hv_timing(_config->hw.mode,
+ drm_mode_get_hv_timing(_config->hw.adjusted_mode,
_config->pipe_src_w,
_config->pipe_src_h);
@@ -18461,15 +18462,11 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
int min_cdclk = 0;
if (crtc_state->hw.active) {
- struct drm_display_mode *mode = _state->hw.mode;
+ struct drm_display_mode mode;
intel_mode_from_pipe_config(_state->hw.adjusted_mode,
crtc_state);
- *mode = crtc_state->hw.adjusted_mode;
- mode->hdisplay = crtc_state->pipe_src_w;
- mode->vdisplay = crtc_state->pipe_src_h;
-
/*
* The initial mode needs to be set in order to keep
* the atomic core happy. It wants a valid mode if the
@@ -18481,11 +18478,15 @@ static void intel_modeset_readout_hw_state(struct
drm_device *dev)
*/
crtc_state->inherited = true;
+ mode = crtc_state->hw.adjusted_mode;
+
[Intel-gfx] [PATCH v6 02/11] drm/i915: Remove hw.mode
From: Maarten Lankhorst
The members in hw.mode can be used from adjusted_mode as well,
use that when available.
Some places that use hw.mode can be converted to use adjusted_mode
as well.
v2:
* Manual rebase (Manasi)
* remove the use of pipe_mode defined in patch 3 (Manasi)
v3:
* Rebase on drm-tip (Manasi)
Signed-off-by: Maarten Lankhorst
Signed-off-by: Manasi Navare
---
drivers/gpu/drm/i915/display/intel_display.c | 29 ++-
.../drm/i915/display/intel_display_types.h| 2 +-
drivers/gpu/drm/i915/display/intel_dvo.c | 2 +-
drivers/gpu/drm/i915/display/intel_sdvo.c | 16 --
4 files changed, 23 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 729ec6e0d43a..8652a7c6bf11 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8892,9 +8892,6 @@ static void intel_get_pipe_src_size(struct intel_crtc
*crtc,
tmp = intel_de_read(dev_priv, PIPESRC(crtc->pipe));
pipe_config->pipe_src_h = (tmp & 0x) + 1;
pipe_config->pipe_src_w = ((tmp >> 16) & 0x) + 1;
-
- pipe_config->hw.mode.vdisplay = pipe_config->pipe_src_h;
- pipe_config->hw.mode.hdisplay = pipe_config->pipe_src_w;
}
void intel_mode_from_pipe_config(struct drm_display_mode *mode,
@@ -13079,7 +13076,7 @@ static void intel_dump_pipe_config(const struct
intel_crtc_state *pipe_config,
intel_dump_dp_vsc_sdp(dev_priv, _config->infoframes.vsc);
drm_dbg_kms(_priv->drm, "requested mode:\n");
- drm_mode_debug_printmodeline(_config->hw.mode);
+ drm_mode_debug_printmodeline(_config->uapi.mode);
drm_dbg_kms(_priv->drm, "adjusted mode:\n");
drm_mode_debug_printmodeline(_config->hw.adjusted_mode);
intel_dump_crtc_timings(dev_priv, _config->hw.adjusted_mode);
@@ -13221,17 +13218,17 @@ intel_crtc_copy_uapi_to_hw_state(struct
intel_crtc_state *crtc_state)
{
crtc_state->hw.enable = crtc_state->uapi.enable;
crtc_state->hw.active = crtc_state->uapi.active;
- crtc_state->hw.mode = crtc_state->uapi.mode;
crtc_state->hw.adjusted_mode = crtc_state->uapi.adjusted_mode;
intel_crtc_copy_uapi_to_hw_state_nomodeset(crtc_state);
}
-static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
*crtc_state)
+static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state
*crtc_state,
+struct drm_display_mode *user_mode)
{
crtc_state->uapi.enable = crtc_state->hw.enable;
crtc_state->uapi.active = crtc_state->hw.active;
drm_WARN_ON(crtc_state->uapi.crtc->dev,
- drm_atomic_set_mode_for_crtc(_state->uapi,
_state->hw.mode) < 0);
+ drm_atomic_set_mode_for_crtc(_state->uapi, user_mode)
< 0);
crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
@@ -13277,6 +13274,10 @@ intel_crtc_prepare_cleared_state(struct
intel_crtc_state *crtc_state)
memcpy(crtc_state, saved_state, sizeof(*crtc_state));
kfree(saved_state);
+ /* Clear I915_MODE_FLAG_INHERITED */
+ crtc_state->uapi.mode.private_flags = 0;
+ crtc_state->uapi.adjusted_mode.private_flags = 0;
+
intel_crtc_copy_uapi_to_hw_state(crtc_state);
return 0;
@@ -13324,7 +13325,7 @@ intel_modeset_pipe_config(struct intel_crtc_state
*pipe_config)
* computation to clearly distinguish it from the adjusted mode, which
* can be changed by the connectors in the below retry loop.
*/
- drm_mode_get_hv_timing(_config->hw.mode,
+ drm_mode_get_hv_timing(_config->hw.adjusted_mode,
_config->pipe_src_w,
_config->pipe_src_h);
@@ -18461,15 +18462,11 @@ static void intel_modeset_readout_hw_state(struct
drm_device *dev)
int min_cdclk = 0;
if (crtc_state->hw.active) {
- struct drm_display_mode *mode = _state->hw.mode;
+ struct drm_display_mode mode;
intel_mode_from_pipe_config(_state->hw.adjusted_mode,
crtc_state);
- *mode = crtc_state->hw.adjusted_mode;
- mode->hdisplay = crtc_state->pipe_src_w;
- mode->vdisplay = crtc_state->pipe_src_h;
-
/*
* The initial mode needs to be set in order to keep
* the atomic core happy. It wants a valid mode if the
@@ -18481,11 +18478,15 @@ static void intel_modeset_readout_hw_state(struct
drm_device *dev)
*/
crtc_state->inherited = true;
+ mode = crtc_state->hw.adjusted_mode;
+ mode.hdisplay =
