2013/8/29 Paulo Zanoni <przanoni@xxxxxxxxx>:
> Hi
>
> 2013/8/23 Jani Nikula <jani.nikula@xxxxxxxxx>:
>> The bios interface seems messy, and it's hard to tell what the bios
>> really wants. At first, only add support for DDI based machines (hsw+),
>> and see how it turns out.
>>
>> The spec says to notify prior to power down and after power up. It is
>> unclear whether it makes a difference.
>>
>> v2:
>> - squash notification function and callers patches together (Daniel)
>> - move callers to haswell_crtc_{enable,disable} (Daniel)
>> - rename notification function (Chris)
>>
>> v3:
>> - separate notification function and callers again, as it's not clear
>> whether the display power state notification is the right thing to do
>> after all
>>
>> Signed-off-by: Jani Nikula <jani.nikula@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 8 +++++
>> drivers/gpu/drm/i915/intel_opregion.c | 52 +++++++++++++++++++++++++++++++++
>> 2 files changed, 60 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index adc2f46..1703029 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2171,15 +2171,23 @@ static inline bool intel_gmbus_is_forced_bit(struct i2c_adapter *adapter)
>> extern void intel_i2c_reset(struct drm_device *dev);
>>
>> /* intel_opregion.c */
>> +struct intel_encoder;
>> extern int intel_opregion_setup(struct drm_device *dev);
>> #ifdef CONFIG_ACPI
>> extern void intel_opregion_init(struct drm_device *dev);
>> extern void intel_opregion_fini(struct drm_device *dev);
>> extern void intel_opregion_asle_intr(struct drm_device *dev);
>> +extern int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>> + bool enable);
>> #else
>> static inline void intel_opregion_init(struct drm_device *dev) { return; }
>> static inline void intel_opregion_fini(struct drm_device *dev) { return; }
>> static inline void intel_opregion_asle_intr(struct drm_device *dev) { return; }
>> +static inline int
>> +intel_opregion_notify_encoder(struct intel_encoder *intel_encoder, bool enable)
>> +{
>> + return 0;
>> +}
>> #endif
>>
>> /* intel_acpi.c */
>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>> index a3558ae..72a4af6 100644
>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>> @@ -273,6 +273,58 @@ static int swsci(struct drm_device *dev, u32 function, u32 parm, u32 *parm_out)
>> #undef C
>> }
>>
>> +#define DISPLAY_TYPE_CRT 0
>> +#define DISPLAY_TYPE_TV 1
>> +#define DISPLAY_TYPE_EXTERNAL_FLAT_PANEL 2
>> +#define DISPLAY_TYPE_INTERNAL_FLAT_PANEL 3
>> +
>> +int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>> + bool enable)
>> +{
>> + struct drm_device *dev = intel_encoder->base.dev;
>> + u32 parm = 0;
>> + u32 type = 0;
>> + u32 port;
>> +
>> + /* don't care about old stuff for now */
>> + if (!HAS_DDI(dev))
>> + return 0;
>> +
>> + port = intel_ddi_get_encoder_port(intel_encoder);
>> + if (port == PORT_E) {
>> + port = 0;
>> + } else {
>> + parm |= 1 << port;
>> + port++;
>> + }
>> +
>> + if (!enable)
>> + parm |= 4 << 8;
>> +
>> + switch (intel_encoder->type) {
>> + case INTEL_OUTPUT_ANALOG:
>> + type = DISPLAY_TYPE_CRT;
>> + break;
>> + case INTEL_OUTPUT_UNKNOWN:
>> + case INTEL_OUTPUT_DISPLAYPORT:
>> + case INTEL_OUTPUT_HDMI:
>> + type = DISPLAY_TYPE_EXTERNAL_FLAT_PANEL;
>> + break;
>> + case INTEL_OUTPUT_LVDS:
>
> We don't have LVDS on DDI platforms.
>
>
>> + case INTEL_OUTPUT_EDP:
>> + type = DISPLAY_TYPE_INTERNAL_FLAT_PANEL;
>> + break;
>> + default:
>> + DRM_DEBUG_DRIVER("unsupported intel_encoder type %d\n",
>> + intel_encoder->type);
This should be a WARN.
>> + return -EINVAL;
>> + }
>> +
>> + parm |= type << (16 + port * 3);
>> +
>> + return swsci(dev, SWSCI_SBCB_DISPLAY_POWER_STATE, parm, NULL);
>
> In patch 2, on the initialization code you call the GBDA request
> callbacks function to see which ones will work. Shouldn't you also add
> code to call the SBCB request callbacks function and see if
> DISPLAY_POWER_STATE is really supported? I know that
> DISPLAY_POWER_STATE is supposed to be mandatory, but just checking
> won't hurt us. We could maybe even add a code to WARN in case one of
> the mandatory callbacks is not available :) This suggestion could be
> in a separate patch.
>
> With or without any changes: Reviewed-by: Paulo Zanoni
> <paulo.r.zanoni@xxxxxxxxx>
>
>
>> +}
>> +
>> static u32 asle_set_backlight(struct drm_device *dev, u32 bclp)
>> {
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> --
>> 1.7.9.5
>>
>
>
>
> --
> Paulo Zanoni
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
