Commit 5ab432ef4997ce32c9406721b37ef6e97e57dae1

Authored by Daniel Vetter
1 parent ef9c3aee60

drm/i915/hdmi: convert to encoder->disable/enable

I've picked hdmi as the first encoder to convert because it's rather
simple:
- no cloning possible
- no differences between prepare/commit and dpms off/on switching.

A few changes are required to do so:
- Split up the dpms code into an enable/disable function and wire it
  up with the intel encoder.
- Noop out the existing encoder prepare/commit functions used by the
  crtc helper - our crtc enable/disable code now calls back into the
  encoder enable/disable code at the right spot.
- Create new helper functions to handle dpms changes.
- Add intel_encoder->connectors_active to better track dpms state. Atm
  this is unused, but it will be useful to correctly disable the
  entire display pipe for cloned configurations. Also note that for
  now this is only useful in the dpms code - thanks to the crtc
  helper's dpms confusion across a modeset operation we can't (yet)
  rely on this having a sensible value in all circumstances.
- Rip out the encoder helper dpms callback, if this is still getting
  called somewhere we have a bug. The slight issue with that is that
  the crtc helper abuses dpms off to disable unused functions. Hence
  we also need to implement a default encoder disable function to do
  just that with the new encoder->disable callback.
- Note that we drop the cpt modeset verification in the commit
  callback, too. The right place to do this would be in the crtc's
  enable function, _after_ all the encoders are set up. But because
  not all encoders are converted yet, we can't do that. Hence disable
  this check temporarily as a minor concession to bisectability.

v2: Squash the dpms mode to only the supported values -
connector->dpms is for internal tracking only, we can hence avoid
needless state-changes a bit whithout causing harm.

v3: Apply bikeshed to disable|enable_ddi, suggested by Paulo Zanoni.

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Showing 4 changed files with 160 additions and 53 deletions Side-by-side Diff

drivers/gpu/drm/i915/intel_ddi.c
... ... @@ -757,27 +757,35 @@
757 757 intel_hdmi->set_infoframes(encoder, adjusted_mode);
758 758 }
759 759  
760   -void intel_ddi_dpms(struct drm_encoder *encoder, int mode)
  760 +void intel_enable_ddi(struct intel_encoder *encoder)
761 761 {
762   - struct drm_device *dev = encoder->dev;
  762 + struct drm_device *dev = encoder->base.dev;
763 763 struct drm_i915_private *dev_priv = dev->dev_private;
764   - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
  764 + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
765 765 int port = intel_hdmi->ddi_port;
766 766 u32 temp;
767 767  
768 768 temp = I915_READ(DDI_BUF_CTL(port));
  769 + temp |= DDI_BUF_CTL_ENABLE;
769 770  
770   - if (mode != DRM_MODE_DPMS_ON) {
771   - temp &= ~DDI_BUF_CTL_ENABLE;
772   - } else {
773   - temp |= DDI_BUF_CTL_ENABLE;
774   - }
775   -
776 771 /* Enable DDI_BUF_CTL. In HDMI/DVI mode, the port width,
777 772 * and swing/emphasis values are ignored so nothing special needs
778 773 * to be done besides enabling the port.
779 774 */
780   - I915_WRITE(DDI_BUF_CTL(port),
781   - temp);
  775 + I915_WRITE(DDI_BUF_CTL(port), temp);
  776 +}
  777 +
  778 +void intel_disable_ddi(struct intel_encoder *encoder)
  779 +{
  780 + struct drm_device *dev = encoder->base.dev;
  781 + struct drm_i915_private *dev_priv = dev->dev_private;
  782 + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
  783 + int port = intel_hdmi->ddi_port;
  784 + u32 temp;
  785 +
  786 + temp = I915_READ(DDI_BUF_CTL(port));
  787 + temp &= ~DDI_BUF_CTL_ENABLE;
  788 +
  789 + I915_WRITE(DDI_BUF_CTL(port), temp);
782 790 }
drivers/gpu/drm/i915/intel_display.c
... ... @@ -3554,12 +3554,61 @@
3554 3554 intel_cpt_verify_modeset(dev, intel_crtc->pipe);
3555 3555 }
3556 3556  
  3557 +void intel_encoder_noop(struct drm_encoder *encoder)
  3558 +{
  3559 +}
  3560 +
  3561 +void intel_encoder_disable(struct drm_encoder *encoder)
  3562 +{
  3563 + struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
  3564 +
  3565 + intel_encoder->disable(intel_encoder);
  3566 +}
  3567 +
3557 3568 void intel_encoder_destroy(struct drm_encoder *encoder)
3558 3569 {
3559 3570 struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
3560 3571  
3561 3572 drm_encoder_cleanup(encoder);
3562 3573 kfree(intel_encoder);
  3574 +}
  3575 +
  3576 +/* Simple dpms helper for encodres with just one connector, no cloning and only
  3577 + * one kind of off state. It clamps all !ON modes to fully OFF and changes the
  3578 + * state of the entire output pipe. */
  3579 +void intel_encoder_dpms(struct intel_encoder *encoder, int mode)
  3580 +{
  3581 + if (mode == DRM_MODE_DPMS_ON) {
  3582 + encoder->connectors_active = true;
  3583 +
  3584 + intel_crtc_dpms(encoder->base.crtc, DRM_MODE_DPMS_ON);
  3585 + } else {
  3586 + encoder->connectors_active = false;
  3587 +
  3588 + intel_crtc_dpms(encoder->base.crtc, DRM_MODE_DPMS_OFF);
  3589 + }
  3590 +}
  3591 +
  3592 +/* Even simpler default implementation, if there's really no special case to
  3593 + * consider. */
  3594 +void intel_connector_dpms(struct drm_connector *connector, int mode)
  3595 +{
  3596 + struct intel_encoder *encoder = intel_attached_encoder(connector);
  3597 +
  3598 + /* All the simple cases only support two dpms states. */
  3599 + if (mode != DRM_MODE_DPMS_ON)
  3600 + mode = DRM_MODE_DPMS_OFF;
  3601 +
  3602 + if (mode == connector->dpms)
  3603 + return;
  3604 +
  3605 + connector->dpms = mode;
  3606 +
  3607 + /* Only need to change hw state when actually enabled */
  3608 + if (encoder->base.crtc)
  3609 + intel_encoder_dpms(encoder, mode);
  3610 + else
  3611 + encoder->connectors_active = false;
3563 3612 }
3564 3613  
3565 3614 static bool intel_crtc_mode_fixup(struct drm_crtc *crtc,
drivers/gpu/drm/i915/intel_drv.h
... ... @@ -140,6 +140,7 @@
140 140 * simple flag is enough to compute the possible_clones mask.
141 141 */
142 142 bool cloneable;
  143 + bool connectors_active;
143 144 void (*hot_plug)(struct intel_encoder *);
144 145 void (*enable)(struct intel_encoder *);
145 146 void (*disable)(struct intel_encoder *);
146 147  
... ... @@ -412,7 +413,11 @@
412 413 extern void intel_crtc_load_lut(struct drm_crtc *crtc);
413 414 extern void intel_encoder_prepare(struct drm_encoder *encoder);
414 415 extern void intel_encoder_commit(struct drm_encoder *encoder);
  416 +extern void intel_encoder_noop(struct drm_encoder *encoder);
  417 +extern void intel_encoder_disable(struct drm_encoder *encoder);
415 418 extern void intel_encoder_destroy(struct drm_encoder *encoder);
  419 +extern void intel_encoder_dpms(struct intel_encoder *encoder, int mode);
  420 +extern void intel_connector_dpms(struct drm_connector *, int mode);
416 421  
417 422 static inline struct intel_encoder *intel_attached_encoder(struct drm_connector *connector)
418 423 {
... ... @@ -519,7 +524,8 @@
519 524 extern void gen6_gt_check_fifodbg(struct drm_i915_private *dev_priv);
520 525 extern void ironlake_teardown_rc6(struct drm_device *dev);
521 526  
522   -extern void intel_ddi_dpms(struct drm_encoder *encoder, int mode);
  527 +extern void intel_enable_ddi(struct intel_encoder *encoder);
  528 +extern void intel_disable_ddi(struct intel_encoder *encoder);
523 529 extern void intel_ddi_mode_set(struct drm_encoder *encoder,
524 530 struct drm_display_mode *mode,
525 531 struct drm_display_mode *adjusted_mode);
drivers/gpu/drm/i915/intel_hdmi.c
... ... @@ -601,11 +601,11 @@
601 601 intel_hdmi->set_infoframes(encoder, adjusted_mode);
602 602 }
603 603  
604   -static void intel_hdmi_dpms(struct drm_encoder *encoder, int mode)
  604 +static void intel_enable_hdmi(struct intel_encoder *encoder)
605 605 {
606   - struct drm_device *dev = encoder->dev;
  606 + struct drm_device *dev = encoder->base.dev;
607 607 struct drm_i915_private *dev_priv = dev->dev_private;
608   - struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
  608 + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
609 609 u32 temp;
610 610 u32 enable_bits = SDVO_ENABLE;
611 611  
612 612  
613 613  
614 614  
... ... @@ -617,30 +617,70 @@
617 617 /* HW workaround for IBX, we need to move the port to transcoder A
618 618 * before disabling it. */
619 619 if (HAS_PCH_IBX(dev)) {
620   - struct drm_crtc *crtc = encoder->crtc;
  620 + struct drm_crtc *crtc = encoder->base.crtc;
621 621 int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1;
622 622  
623   - if (mode != DRM_MODE_DPMS_ON) {
624   - if (temp & SDVO_PIPE_B_SELECT) {
625   - temp &= ~SDVO_PIPE_B_SELECT;
626   - I915_WRITE(intel_hdmi->sdvox_reg, temp);
627   - POSTING_READ(intel_hdmi->sdvox_reg);
  623 + /* Restore the transcoder select bit. */
  624 + if (pipe == PIPE_B)
  625 + enable_bits |= SDVO_PIPE_B_SELECT;
  626 + }
628 627  
629   - /* Again we need to write this twice. */
630   - I915_WRITE(intel_hdmi->sdvox_reg, temp);
631   - POSTING_READ(intel_hdmi->sdvox_reg);
  628 + /* HW workaround, need to toggle enable bit off and on for 12bpc, but
  629 + * we do this anyway which shows more stable in testing.
  630 + */
  631 + if (HAS_PCH_SPLIT(dev)) {
  632 + I915_WRITE(intel_hdmi->sdvox_reg, temp & ~SDVO_ENABLE);
  633 + POSTING_READ(intel_hdmi->sdvox_reg);
  634 + }
632 635  
633   - /* Transcoder selection bits only update
634   - * effectively on vblank. */
635   - if (crtc)
636   - intel_wait_for_vblank(dev, pipe);
637   - else
638   - msleep(50);
639   - }
640   - } else {
641   - /* Restore the transcoder select bit. */
642   - if (pipe == PIPE_B)
643   - enable_bits |= SDVO_PIPE_B_SELECT;
  636 + temp |= enable_bits;
  637 +
  638 + I915_WRITE(intel_hdmi->sdvox_reg, temp);
  639 + POSTING_READ(intel_hdmi->sdvox_reg);
  640 +
  641 + /* HW workaround, need to write this twice for issue that may result
  642 + * in first write getting masked.
  643 + */
  644 + if (HAS_PCH_SPLIT(dev)) {
  645 + I915_WRITE(intel_hdmi->sdvox_reg, temp);
  646 + POSTING_READ(intel_hdmi->sdvox_reg);
  647 + }
  648 +}
  649 +
  650 +static void intel_disable_hdmi(struct intel_encoder *encoder)
  651 +{
  652 + struct drm_device *dev = encoder->base.dev;
  653 + struct drm_i915_private *dev_priv = dev->dev_private;
  654 + struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
  655 + u32 temp;
  656 + u32 enable_bits = SDVO_ENABLE;
  657 +
  658 + if (intel_hdmi->has_audio)
  659 + enable_bits |= SDVO_AUDIO_ENABLE;
  660 +
  661 + temp = I915_READ(intel_hdmi->sdvox_reg);
  662 +
  663 + /* HW workaround for IBX, we need to move the port to transcoder A
  664 + * before disabling it. */
  665 + if (HAS_PCH_IBX(dev)) {
  666 + struct drm_crtc *crtc = encoder->base.crtc;
  667 + int pipe = crtc ? to_intel_crtc(crtc)->pipe : -1;
  668 +
  669 + if (temp & SDVO_PIPE_B_SELECT) {
  670 + temp &= ~SDVO_PIPE_B_SELECT;
  671 + I915_WRITE(intel_hdmi->sdvox_reg, temp);
  672 + POSTING_READ(intel_hdmi->sdvox_reg);
  673 +
  674 + /* Again we need to write this twice. */
  675 + I915_WRITE(intel_hdmi->sdvox_reg, temp);
  676 + POSTING_READ(intel_hdmi->sdvox_reg);
  677 +
  678 + /* Transcoder selection bits only update
  679 + * effectively on vblank. */
  680 + if (crtc)
  681 + intel_wait_for_vblank(dev, pipe);
  682 + else
  683 + msleep(50);
644 684 }
645 685 }
646 686  
... ... @@ -652,11 +692,7 @@
652 692 POSTING_READ(intel_hdmi->sdvox_reg);
653 693 }
654 694  
655   - if (mode != DRM_MODE_DPMS_ON) {
656   - temp &= ~enable_bits;
657   - } else {
658   - temp |= enable_bits;
659   - }
  695 + temp &= ~enable_bits;
660 696  
661 697 I915_WRITE(intel_hdmi->sdvox_reg, temp);
662 698 POSTING_READ(intel_hdmi->sdvox_reg);
663 699  
664 700  
665 701  
666 702  
667 703  
668 704  
... ... @@ -849,23 +885,23 @@
849 885 }
850 886  
851 887 static const struct drm_encoder_helper_funcs intel_hdmi_helper_funcs_hsw = {
852   - .dpms = intel_ddi_dpms,
853 888 .mode_fixup = intel_hdmi_mode_fixup,
854   - .prepare = intel_encoder_prepare,
  889 + .prepare = intel_encoder_noop,
855 890 .mode_set = intel_ddi_mode_set,
856   - .commit = intel_encoder_commit,
  891 + .commit = intel_encoder_noop,
  892 + .disable = intel_encoder_disable,
857 893 };
858 894  
859 895 static const struct drm_encoder_helper_funcs intel_hdmi_helper_funcs = {
860   - .dpms = intel_hdmi_dpms,
861 896 .mode_fixup = intel_hdmi_mode_fixup,
862   - .prepare = intel_encoder_prepare,
  897 + .prepare = intel_encoder_noop,
863 898 .mode_set = intel_hdmi_mode_set,
864   - .commit = intel_encoder_commit,
  899 + .commit = intel_encoder_noop,
  900 + .disable = intel_encoder_disable,
865 901 };
866 902  
867 903 static const struct drm_connector_funcs intel_hdmi_connector_funcs = {
868   - .dpms = drm_helper_connector_dpms,
  904 + .dpms = intel_connector_dpms,
869 905 .detect = intel_hdmi_detect,
870 906 .fill_modes = drm_helper_probe_single_connector_modes,
871 907 .set_property = intel_hdmi_set_property,
... ... @@ -964,10 +1000,18 @@
964 1000 intel_hdmi->set_infoframes = cpt_set_infoframes;
965 1001 }
966 1002  
967   - if (IS_HASWELL(dev))
968   - drm_encoder_helper_add(&intel_encoder->base, &intel_hdmi_helper_funcs_hsw);
969   - else
970   - drm_encoder_helper_add(&intel_encoder->base, &intel_hdmi_helper_funcs);
  1003 + if (IS_HASWELL(dev)) {
  1004 + intel_encoder->enable = intel_enable_ddi;
  1005 + intel_encoder->disable = intel_disable_ddi;
  1006 + drm_encoder_helper_add(&intel_encoder->base,
  1007 + &intel_hdmi_helper_funcs_hsw);
  1008 + } else {
  1009 + intel_encoder->enable = intel_enable_hdmi;
  1010 + intel_encoder->disable = intel_disable_hdmi;
  1011 + drm_encoder_helper_add(&intel_encoder->base,
  1012 + &intel_hdmi_helper_funcs);
  1013 + }
  1014 +
971 1015  
972 1016 intel_hdmi_add_properties(intel_hdmi, connector);
973 1017