On 09-Oct-14 9:17 PM, Daniel Vetter wrote:
On Tue, Oct 07, 2014 at 08:39:38PM +0530, Vandana Kannan wrote:
Since panel power sequencing is a feature common to all internal displays,
moving relevant code to intel_panel.c. This patch series contains changes
to setup PPS data and program register values as required.
The implementation follows the model used for backlight funcs
(as suggested by Daniel) which splits the changes based on platform.
As of now, changes have been made considering only eDP.
TODO:-
1. To accomodate software delays where applicable, placeholders have been
created in i915_display_funcs, but have not been defined yet.
2. Integrate MIPI PPS delays once 1. is done.
3. PPS delays would be required for LVDS as well. The existing file
intel_lvds.c does not make use of the delays.
I haven't taken a in-depth look at your patches since I'm travelling.
But one thing I've noticed while scrolling through them is that you
first add most of the new code in a few patches, and then remove the
old one in follow-up patches. This makes reviewing patches a lot
harder.
A better way to split refactoring patch series is to do the split
per-function. So for this series here maybe have a patch for the pps
init changes, the functions to enable/disable power, and so on. And
the important part is that you add/remove/change the code in one patch
for a given function so that the actual change can be reviewed. So the
new vtable functions should grow out of the existing code.
And if you need to split code this should always be done in 2 steps:
First make a verbatim copy with the new names, then refactor both
copies to be platform specific. Of course if the code you copy is just
a few lines that can be done in one step, but as soon as you can't
read both functions completely in the diff context you should do the
split. Jani has definitely overstretched the limit with his bachlight
patches, but occasionally I let things slip through ;-)
Yours, Daniel
Hi Daniel,
Thanks for your inputs.
Agree with your comments, will make changes to the patches accordingly
and resend.
Thanks,
Vandana
Vandana Kannan (6):
drm/i915: Create PPS related struct and func pointers
drm/i915: Define PPS setup functions
drm/i915: Program PPS registers
drm/i915: Removing refs to intel_dp_panel_power_sequencer
drm/i915: Replace all refs to intel_dp delays
drm/i915: Modify refs to intel dp timestamps
drivers/gpu/drm/i915/i915_drv.h | 15 ++
drivers/gpu/drm/i915/intel_display.c | 1 +
drivers/gpu/drm/i915/intel_dp.c | 275 ++++++-----------------------------
drivers/gpu/drm/i915/intel_drv.h | 32 +++-
drivers/gpu/drm/i915/intel_panel.c | 260 +++++++++++++++++++++++++++++++++
5 files changed, 346 insertions(+), 237 deletions(-)
--
2.0.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
