Re: [PATCH 00/89] Basic Skylake enabling (reviewers)

Thomas Wood
2014-09-16T14:52:02Z

On 4 September 2014 15:16, Damien Lespiau <damien.lespiau@intel.com> wrote:
> Of course, the series now needs reviewers. There's a list of known
> problems that I'm planning to address, a few of those problems are easy
> to solve and ca be addressed as a new revision of those pathes. However
> I'm hoping other ones can be follup up patches instead.
>
>
> Known issues:
>
> - There a known limitation in the DDB allocation code. It doesn't
> respect the minimal allocation of 8 blocks. When we're trying to
> scannout two planes (not counting the cursor) that have widely
> different data rates (1080p and 64x64), we'll under-allocate the
> small plane and hit underuns.
>
> (follow up series)
>
> - There are a fair number of patches that are just "|| IS_GEN9(dev)".
> I'd do a "let's by optimistic" pass on the driver to turn those into
> "gen >= 9" to try to limit the number of small patches in the
> future.
>
> (follow up series)
>
> - I haven't done a pass of the W/A just yet, waiting for things to
> settle a bit. In particular I haven't checked that the few W/A
> implemented can be done in init_clock_gating() or needs to be LRIs.
> (the patch predates all of that).
>
> (follow up series)
>
> - While looking at the patches before sending, I've noticed some
> extraneous defines in "Fix the extra defines in "drm/i915/skl: Read
> the Memory Latency Values for WM computation".
>
> Will send a new version of this patch rigth away
>
> - There's potential to unify the primary and sprite planes functions
> now that the primary plane is just another plane. This needs a bit
> of work to unify those paths.
>
> My current plan is to address this as a follow up series, not high
> priority.
>
> - Daniel had a few comments piped up already, the bigger ones have been
> addressed. I, however didn't look at this one
>
> < danvet> edp unconditionally uses cdclk/dpll0
> < danvet> but we don't track the port clock for that anywhere in the pipe
> config
>
>
> Reviewers
>
> some of the patches already have a r-b tag, having another look has some
> value as patches get rebased and churned a bit. Some people have 2 lines
> below.
>
> I've left some people out of the list as they're just jumping on the
> list and give feedback already. If you expect some big delays in the
> review, please do speak up so I can try to find somebody else.
>
> Patches 1 to 20, excluding 12: Thomas Wood

Patches 1 to 5, 7, 9, 13 to 16, 18 and 19 are:

Reviewed-by: Thomas Wood <thomas.wood@intel.com>

There are a few minor issues with patches 6 and 10:

Patch 6 has a "v1" in the commit message, rather than "v2".
Patch 10 seems to reference a non-existent commit id in the commit message.

Otherwise, these are also:

Reviewed-by: Thomas Wood <thomas.wood@intel.com>

Patches 8 and 11 have further comments, and patch 17 can be dropped as
it is superseded by patch 21.

Patch 20 is already reviewed and I am unsure if there was a conclusion
from the previous discussion about this change.

>
> Patches 12 + 21 to 42: Rodrigo
>
> Patches 43 to 52: Ville (WM)
>
> Patches 53 to 56: Mika (forcewake engine + a bit of rc6)
>
> Patch 57 is already merged
>
> Patches 58 to 68: Paulo (DPLL + removal of the eDP training W/A)
>
> Patches 69 to 73: Imre (Power wells)
>
> Patches 74 to 75: Paulo (queue_flip, pfit)
>
> Patches 76 to 83: Ville (WM part2)
>
> Patch 84 to 89: Mika (turbo and various small patches, some of them already
> dropped and/or reviewed)
>
> Thanks,
>
> --
> Damien