Re: [Intel-gfx] [PATCH 0/7] modeset rework prep patches
On Sun, Aug 12, 2012 at 08:47:56PM +0100, Chris Wilson wrote: On Sun, 12 Aug 2012 19:27:07 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: Hi all, I've noticed that a few prep patches of the modeset rework series haven't been merged nor reviewed yet, so I've split them out in this resend. Mostly concern really old gen2 stuff (dvo + pipe A quirk), but little patches in other areas. Commentsreview highly welcome. Thanks, Daniel Daniel Vetter (7): drm/i915: add missing gen2 pipe A quirk entries I remain dubious whether the 855gm entry is genuine. drm/i915/ns2501: kill pll A enabling hack drm/i915: rip out the overlay pipe A workaround Look fine and a welcome reduction in code + confusion. drm/i915: prepare load-detect pipe code for dpms changes It is not immediately obvious from the function that there is a relationship between the connector and intel_encoder. If we derived the encoder from the connector in that function, the reviewer's life gets a little easier. As it stands the code looks correct and rightly removes some internal details. drm/i915: simplify dvo dpms interface This just looks like churn for churn's sake? The changes look correct. drm/i915: kill a few unused things in dev_priv +1 drm/i915: extract ironlake_fdi_pll_disable Smaller more descriptive functions, what is not to like. 2-7: Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk You can have an r-b for 1 if you drop the 855gm chunk. Then get someone else to a-b the 855gm entry :) I've slurped in the revised series, thanks a lot for the review. -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/7] modeset rework prep patches
Hi all, I've noticed that a few prep patches of the modeset rework series haven't been merged nor reviewed yet, so I've split them out in this resend. Mostly concern really old gen2 stuff (dvo + pipe A quirk), but little patches in other areas. Commentsreview highly welcome. Thanks, Daniel Daniel Vetter (7): drm/i915: add missing gen2 pipe A quirk entries drm/i915/ns2501: kill pll A enabling hack drm/i915: rip out the overlay pipe A workaround drm/i915: prepare load-detect pipe code for dpms changes drm/i915: simplify dvo dpms interface drm/i915: kill a few unused things in dev_priv drm/i915: extract ironlake_fdi_pll_disable drivers/gpu/drm/i915/dvo.h | 9 ++- drivers/gpu/drm/i915/dvo_ch7017.c| 8 +-- drivers/gpu/drm/i915/dvo_ch7xxx.c| 4 +- drivers/gpu/drm/i915/dvo_ivch.c | 8 +-- drivers/gpu/drm/i915/dvo_ns2501.c| 21 ++- drivers/gpu/drm/i915/dvo_sil164.c| 4 +- drivers/gpu/drm/i915/dvo_tfp410.c| 4 +- drivers/gpu/drm/i915/i915_dma.c | 22 drivers/gpu/drm/i915/i915_drv.h | 13 ++--- drivers/gpu/drm/i915/intel_display.c | 105 +-- drivers/gpu/drm/i915/intel_dvo.c | 4 +- drivers/gpu/drm/i915/intel_overlay.c | 58 +-- 12 files changed, 94 insertions(+), 166 deletions(-) -- 1.7.11.2 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/7] modeset rework prep patches
On Sun, 12 Aug 2012 19:27:07 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: Hi all, I've noticed that a few prep patches of the modeset rework series haven't been merged nor reviewed yet, so I've split them out in this resend. Mostly concern really old gen2 stuff (dvo + pipe A quirk), but little patches in other areas. Commentsreview highly welcome. Thanks, Daniel Daniel Vetter (7): drm/i915: add missing gen2 pipe A quirk entries I remain dubious whether the 855gm entry is genuine. drm/i915/ns2501: kill pll A enabling hack drm/i915: rip out the overlay pipe A workaround Look fine and a welcome reduction in code + confusion. drm/i915: prepare load-detect pipe code for dpms changes It is not immediately obvious from the function that there is a relationship between the connector and intel_encoder. If we derived the encoder from the connector in that function, the reviewer's life gets a little easier. As it stands the code looks correct and rightly removes some internal details. drm/i915: simplify dvo dpms interface This just looks like churn for churn's sake? The changes look correct. drm/i915: kill a few unused things in dev_priv +1 drm/i915: extract ironlake_fdi_pll_disable Smaller more descriptive functions, what is not to like. 2-7: Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk You can have an r-b for 1 if you drop the 855gm chunk. Then get someone else to a-b the 855gm entry :) -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/7] modeset rework prep patches
On Sun, Aug 12, 2012 at 08:47:56PM +0100, Chris Wilson wrote: On Sun, 12 Aug 2012 19:27:07 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: Hi all, I've noticed that a few prep patches of the modeset rework series haven't been merged nor reviewed yet, so I've split them out in this resend. Mostly concern really old gen2 stuff (dvo + pipe A quirk), but little patches in other areas. Commentsreview highly welcome. Thanks, Daniel Daniel Vetter (7): drm/i915: add missing gen2 pipe A quirk entries I remain dubious whether the 855gm entry is genuine. Ok, since we don't have a bug to support this, I'll drop it again. drm/i915/ns2501: kill pll A enabling hack drm/i915: rip out the overlay pipe A workaround Look fine and a welcome reduction in code + confusion. drm/i915: prepare load-detect pipe code for dpms changes It is not immediately obvious from the function that there is a relationship between the connector and intel_encoder. If we derived the encoder from the connector in that function, the reviewer's life gets a little easier. As it stands the code looks correct and rightly removes some internal details. Hm, good point. I'll add a patch on top that drops the intel_encoder argument (since it's redudant, all callers get it with intel_attached_encoder). Or better if I squash it together with this one? drm/i915: simplify dvo dpms interface This just looks like churn for churn's sake? The changes look correct. We don't bother with anything else than dpms on/off states in most of the modeset code (even for crt newer hw drops the intermediate states). Hence the new interfaces have only enable/disable functions at the encoder/crtc level. I've figured it looks odd if we keep the full dpms interface for dvo. But since it's rather independant churn I've moved it into this odds bits series. drm/i915: kill a few unused things in dev_priv +1 drm/i915: extract ironlake_fdi_pll_disable Smaller more descriptive functions, what is not to like. 2-7: Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk You can have an r-b for 1 if you drop the 855gm chunk. Then get someone else to a-b the 855gm entry :) Thanks, Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/7] modeset rework prep patches
On Sun, 12 Aug 2012 22:01:02 +0200, Daniel Vetter dan...@ffwll.ch wrote: On Sun, Aug 12, 2012 at 08:47:56PM +0100, Chris Wilson wrote: On Sun, 12 Aug 2012 19:27:07 +0200, Daniel Vetter daniel.vet...@ffwll.ch wrote: drm/i915: prepare load-detect pipe code for dpms changes It is not immediately obvious from the function that there is a relationship between the connector and intel_encoder. If we derived the encoder from the connector in that function, the reviewer's life gets a little easier. As it stands the code looks correct and rightly removes some internal details. Hm, good point. I'll add a patch on top that drops the intel_encoder argument (since it's redudant, all callers get it with intel_attached_encoder). Or better if I squash it together with this one? I'll buy it either way, with a slight preference to having it as two steps. drm/i915: simplify dvo dpms interface This just looks like churn for churn's sake? The changes look correct. We don't bother with anything else than dpms on/off states in most of the modeset code (even for crt newer hw drops the intermediate states). Hence the new interfaces have only enable/disable functions at the encoder/crtc level. I've figured it looks odd if we keep the full dpms interface for dvo. But since it's rather independant churn I've moved it into this odds bits series. The full fledged dpms mode isn't going to completely disappear thanks to the CRT dinosaur. I just wonder if we can achieve the same simplification by recognising that all non-zero dpms modes are off, e.g. s/dpms_mode/powersave/ if (powersave) switch_off() else switch_on() -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/7] modeset rework prep patches
On Sun, Aug 12, 2012 at 09:12:14PM +0100, Chris Wilson wrote: On Sun, 12 Aug 2012 22:01:02 +0200, Daniel Vetter dan...@ffwll.ch wrote: On Sun, Aug 12, 2012 at 08:47:56PM +0100, Chris Wilson wrote: drm/i915: simplify dvo dpms interface This just looks like churn for churn's sake? The changes look correct. We don't bother with anything else than dpms on/off states in most of the modeset code (even for crt newer hw drops the intermediate states). Hence the new interfaces have only enable/disable functions at the encoder/crtc level. I've figured it looks odd if we keep the full dpms interface for dvo. But since it's rather independant churn I've moved it into this odds bits series. The full fledged dpms mode isn't going to completely disappear thanks to the CRT dinosaur. I just wonder if we can achieve the same simplification by recognising that all non-zero dpms modes are off, e.g. s/dpms_mode/powersave/ if (powersave) switch_off() else switch_on() Yeah, I'm still handling the crt dinosaur (on hw that supports it) in the modeset rework code. I've just noticed while planning the new modeset code that we handle these dpms codes rather inconsistently. Hence the code has grown a simple bool at the encoder level to track the desired state, which then controls the on/off state of the entire output pipeline (the additional dpms states of the dinosaur are just a nuisance on top of that - every state that requires an active pipes force the entire pipe to be on). This has the advantage that we don't force the pipe on for dpms == standby/suspend just in case we have a crt on it - the new code actually shuts it off if possible. So with the new code the only thing still even dealing with dpms codes is at the connector level. And even there it's just crt (and sdvo, if anyone would fancy writing the patch to enable that ...). -Daniel -- Daniel Vetter Mail: dan...@ffwll.ch Mobile: +41 (0)79 365 57 48 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
