public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code
Date: Fri, 27 Oct 2017 14:49:01 +0300	[thread overview]
Message-ID: <20171027114901.GB10981@intel.com> (raw)
In-Reply-To: <9105c670-18a1-dac6-4554-dfce6003e668@linux.intel.com>

On Fri, Oct 27, 2017 at 01:39:00PM +0200, Maarten Lankhorst wrote:
> Op 19-10-17 om 15:37 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Pass an old crtc state to intel_ddi_post_disable() from the MST code.
> >
> > Note that this crtc state won't necessaitly match the one that was
> > passed to intel_ddi_pre_enable() if the first stream to be enabled isn't
> > the last stream to be disabled. But this is fine since the states should
> > be identical in every important way. This does mean people frobbing
> > the DDI pre_enable/post_disable hooks have to pay attention in what
> > parts of the state they consult.
> >
> > The alternative would be to inline the relevant code into the MST code.
> > That is actually what we used to do for pre_enable before
> > commit e081c8463ac9 ("drm/i915: Remove duplicate DDI enabling logic
> > from MST path"). For post_disable we've always called the DDI hook.
> 
> I woudl rather have it NULL throughout the entire disable sequence, it's going to be
> dangerous anyway, and something dangerous shouldn't be easy. The fact that you have
> to think

It makes for really ugly code as the enable and disable don't look at
all like each other when one has the NULL and the other doesn't. If it's
dangerous for disable, then it's equally dangerous for enable. So I'd
just go with whatever makes the code suck the least, which IMO is
passing a crtc state to both.

Probably the proper fix would be some kind of encoder state which would
be assigned to mst->primary. But that's going to require actual design
work.

> 
> Also there's an unrelated hunk in intel_ddi_pre_enable?

Apparently I wanted to get rid of the local pipe variable.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2017-10-27 11:49 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-19 13:37 [PATCH 00/10] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
2017-10-19 13:37 ` [PATCH 01/10] drm/i915: Don't use encoder->type in intel_ddi_set_pipe_settings() Ville Syrjala
2017-10-19 13:37 ` [PATCH 02/10] drm/i915: Pass crtc state to intel_prepare_dp_ddi_buffers() Ville Syrjala
2017-10-19 13:37 ` [PATCH 03/10] drm/i915: Start using output_types for DPLL selection Ville Syrjala
2017-10-19 13:37 ` [PATCH 04/10] drm/i915: Stop using encoder->type in intel_ddi_enable_transcoder_func() Ville Syrjala
2017-10-19 13:37 ` [PATCH v5 05/10] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
2017-10-27 12:05   ` Maarten Lankhorst
2017-10-27 12:44     ` Ville Syrjälä
2017-10-27 13:53       ` Maarten Lankhorst
2017-10-27 14:03         ` Ville Syrjälä
2017-10-27 14:05           ` Maarten Lankhorst
2017-10-27 19:32     ` Ville Syrjälä
2017-10-19 13:37 ` [PATCH 06/10] drm/i915: Nuke intel_ddi_get_encoder_port() Ville Syrjala
2017-10-19 13:37 ` [PATCH 07/10] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link() Ville Syrjala
2017-10-19 13:37 ` [PATCH 08/10] drm/i915: Pass a crtc state to ddi post_disable from MST code Ville Syrjala
2017-10-27 11:39   ` Maarten Lankhorst
2017-10-27 11:49     ` Ville Syrjälä [this message]
2017-10-27 14:31       ` Maarten Lankhorst
2017-10-19 13:37 ` [PATCH 09/10] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
2017-10-27 11:43   ` Maarten Lankhorst
2017-10-27 11:55     ` Ville Syrjälä
2017-10-19 13:37 ` [PATCH 10/10] drm/i915: Use intel_crtc_has_dp_encoder() for LPE audio Ville Syrjala
2017-10-27 11:43   ` Maarten Lankhorst
2017-10-27 11:53     ` Ville Syrjälä
2017-10-19 14:00 ` ✓ Fi.CI.BAT: success for drm/i915: Last part of DDI encoder->type cleanup Patchwork
2017-10-19 14:59 ` ✓ Fi.CI.IGT: " Patchwork
2017-10-27 16:39 ` ✗ Fi.CI.BAT: failure " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171027114901.GB10981@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=maarten.lankhorst@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox