On Tue, Mar 12, 2013 at 5:37 PM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > 2013/3/12 Rahul Sharma <r.sh.open@xxxxxxxxx>: >> On Mon, Mar 4, 2013 at 7:35 PM, Rahul Sharma <r.sh.open@xxxxxxxxx> wrote: >>> Thanks Sean, >>> >>> On Wed, Feb 27, 2013 at 9:47 PM, Sean Paul <seanpaul@xxxxxxxxxx> wrote: >>>> On Wed, Feb 27, 2013 at 8:22 AM, Rahul Sharma <rahul.sharma@xxxxxxxxxxx> wrote: >>>>> Right now hdmiphy operations and configs are kept inside hdmi driver. hdmiphy related >>>>> code is tightly coupled with hdmi ip driver. Physicaly they are different devices and >>>> >>>> s/Physicaly/Physically/ >>>> >>>>> should be instantiated independently. >>>>> >>>>> In terms of versions/mapping/configurations Hdmi and hdmiphy are independent of each >>>>> other. It is preferred to isolate them and maintained independently. >>>>> >>>>> This implementations is tested for: >>>>> 1) Resolutions supported by exynos4 and 5 hdmi. >>>>> 2) Runtime PM and S2R scenarions for exynos5. >>>>> >>>> >>>> I don't like the idea of spawning off yet another driver in here. It >>>> adds more globals, more suspend/resume ordering issues, and more >>>> implicit dependencies. I understand, however, that this is the Chosen >>>> Way for the exynos driver, so I will save my rant. >>>> >>> >>> I agree to it. splitting phy to a new driver will complicate the power related >>> scenarios. But in upcoming SoC,s, Phy is changing considerably in terms of >>> config values, mapping (i2c/platform bus) etc. Handling this diversity >>> inside hdmi driver is complicating it with unrelated changes. >>> >>> I have tested this RFC for Runtime PM / S2R. But if we see any major roadblock >>> we should re-factor this by explicitly calling power related callbacks >>> of mixer, phy, >>> hdmi drivers in a required order. We can call them from exynos-drm-hdmi plf >>> device. AFAIR something like this is already in place in chrome-kernel. >>> >>>> I've made some comments below. >>>> >>>>> This patch is dependent on >>>>> http://www.mail-archive.com/dri-devel@xxxxxxxxxxxxxxxxxxxxx/msg34733.html >>>>> http://www.mail-archive.com/dri-devel@xxxxxxxxxxxxxxxxxxxxx/msg34861.html >>>>> http://www.mail-archive.com/dri-devel@xxxxxxxxxxxxxxxxxxxxx/msg34862.html >>>>> >>>>> Signed-off-by: Rahul Sharma <rahul.sharma@xxxxxxxxxxx> >>>>> --- >>>>> It is based on exynos-drm-next-todo branch at >>>>> git://git.kernel.org/pub/scm/linux/kernel/git/daeinki/drm-exynos.git >>>>> >>>>> drivers/gpu/drm/exynos/exynos_drm_drv.c | 8 + >>>>> drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 + >>>>> drivers/gpu/drm/exynos/exynos_drm_hdmi.c | 58 ++- >>>>> drivers/gpu/drm/exynos/exynos_drm_hdmi.h | 11 + >>>>> drivers/gpu/drm/exynos/exynos_hdmi.c | 375 ++------------------ >>>>> drivers/gpu/drm/exynos/exynos_hdmi.h | 1 - >>>>> drivers/gpu/drm/exynos/exynos_hdmiphy.c | 586 ++++++++++++++++++++++++++++++- >>>>> drivers/gpu/drm/exynos/regs-hdmiphy.h | 61 ++++ >>>>> 8 files changed, 738 insertions(+), 368 deletions(-) >>>>> create mode 100644 drivers/gpu/drm/exynos/regs-hdmiphy.h >>>>> >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c >>>>> index 3da5c2d..03acb6b 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c >>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c >>>>> @@ -334,6 +334,11 @@ static int __init exynos_drm_init(void) >>>>> ret = platform_driver_register(&hdmi_driver); >>>>> if (ret < 0) >>>>> goto out_hdmi; >>>>> + >>>>> + ret = exynos_hdmiphy_driver_register(); >>>> >>>> Not sure why you need to obfuscate the type of driver here. All the >>>> other ones are directly registered as platform drivers, I don't see >>>> the harm in just doing the i2c_add_driver directly here. >>>> >>> >>> AIMB, Phy is likely to get mapped as a platform device in later soc's. >>> I want to make driver registration (inside exynos_drm_drv.c) independent >>> of this changes. We can handle this inside exynos_hdmiphy_driver_register(). >>> >>>>> + if (ret < 0) >>>>> + goto out_hdmiphy; >>>>> + >>>>> ret = platform_driver_register(&mixer_driver); >>>>> if (ret < 0) >>>>> goto out_mixer; >>>> >>>> >>>> I think this ordering is how you plan on enforcing the suspend/resume >>>> order for hdmi/mixer/hdmiphy. In that case, however, I don't think it >>>> makes sense to suspend/resume hdmiphy in between mixer and hdmi. >>>> Ideally, hdmiphy should go down first and come up last. >>>> >>> >>> I just wanted to keep, hdmi and hdmiphy things together. But what you said >>> above makes sense. I will do that change. >>> >>>> >>>>> @@ -436,6 +441,8 @@ out_common_hdmi_dev: >>>>> out_common_hdmi: >>>>> platform_driver_unregister(&mixer_driver); >>>>> out_mixer: >>>>> + exynos_hdmiphy_driver_unregister(); >>>>> +out_hdmiphy: >>>>> platform_driver_unregister(&hdmi_driver); >>>>> out_hdmi: >>>>> #endif >>>>> @@ -479,6 +486,7 @@ static void __exit exynos_drm_exit(void) >>>>> exynos_platform_device_hdmi_unregister(); >>>>> platform_driver_unregister(&exynos_drm_common_hdmi_driver); >>>>> platform_driver_unregister(&mixer_driver); >>>>> + exynos_hdmiphy_driver_unregister(); >>>>> platform_driver_unregister(&hdmi_driver); >>>>> #endif >>>>> >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h >>>>> index 4606fac..17c53e3 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h >>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h >>>>> @@ -325,6 +325,12 @@ void exynos_drm_subdrv_close(struct drm_device *dev, struct drm_file *file); >>>>> extern int exynos_platform_device_hdmi_register(void); >>>>> >>>>> /* >>>>> + * these functions registers/unregisters exynos drm hdmiphy driver. >>>>> + */ >>>>> +extern int exynos_hdmiphy_driver_register(void); >>>>> +extern void exynos_hdmiphy_driver_unregister(void); >>>>> + >>>>> +/* >>>>> * this function unregisters exynos drm hdmi platform device if it exists. >>>>> */ >>>>> void exynos_platform_device_hdmi_unregister(void); >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c >>>>> index 5dc956b..45ef331 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c >>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c >>>>> @@ -32,19 +32,22 @@ >>>>> /* platform device pointer for common drm hdmi device. */ >>>>> static struct platform_device *exynos_drm_hdmi_pdev; >>>>> >>>>> -/* Common hdmi subdrv needs to access the hdmi and mixer though context. >>>>> -* These should be initialied by the repective drivers */ >>>>> +/* Common hdmi subdrv needs to access the hdmi, hdmiphy and mixer though >>>>> +* context. These should be initialied by the repective drivers */ >>>>> static struct exynos_drm_hdmi_context *hdmi_ctx; >>>>> +static struct exynos_drm_hdmi_context *hdmiphy_ctx; >>>>> static struct exynos_drm_hdmi_context *mixer_ctx; >>>>> >>>>> /* these callback points shoud be set by specific drivers. */ >>>>> static struct exynos_hdmi_ops *hdmi_ops; >>>>> +static struct exynos_hdmiphy_ops *hdmiphy_ops; >>>>> static struct exynos_mixer_ops *mixer_ops; >>>>> >>>> >>>> This stuff really sets my teeth on edge. I can't stand these global >>>> variables and the set-here, set-there, set-here dance that is done on >>>> probe. >>>> >>>> IMO, this and the suspend/resume ordering should be enough motivation >>>> to avoid separating everything out into their own drivers. >>>> >>>> Instead of adding more kruft to this list, we should be working to remove it. >>>> >>>> /rant >>>> >>> >>> Let me think over it. Please suggest me something if you have any idea to >>> correct this. >>> >> >> Hi all, >> >> I replaced these variables with a global LIST of subdrv's with associated >> type (HDMI / PHY / MIXER) and ops struct. >> >> drm-common-hdmi callback will parse this list and based on the subdrv >> type calls subdrv functions, but this adds overhead of parsing the list >> multiple times. >> >> If I cache the subdrvs in drm-common-hdmi context and parsing of the list >> can be avoided. >> >> What you think about above approach? >> > > I think it's good to use existing exynos drm core framework. But I'm > anxious about probing point and existing subdrv callbacks with this. > Could you share that patch as RFC? Mr. Dae, Drm sub-drivers including drm-fimd, drm-common-hdmi will continue using existing exynos drm framework. If I use same for HDMI, PHY and MIXER drivers, I will be bypassing the followed hierarchy i.e. DRM--> EXYNOS-DRM --> Drm-common-hdmi --> (hdmi, mixer, phy). Secondly we need to invoke callbacks in sequence for above three. That condition can not be met if moved to exynos-drm. I modified drm-common-hdmi to create provision for HDMI, PHY and MIXER sub-driver registrations. Then we don't need to maintain so many global variables. If you feel this approach is reasonable I will post the RFC patch. regards, Rahul Sharma. > > Thanks, > Inki Dae > >> regards, >> Rahul Sharma. >> >>>>> struct drm_hdmi_context { >>>>> struct exynos_drm_subdrv subdrv; >>>>> struct exynos_drm_hdmi_context *hdmi_ctx; >>>>> struct exynos_drm_hdmi_context *mixer_ctx; >>>>> + struct exynos_drm_hdmi_context *hdmiphy_ctx; >>>>> >>>>> bool enabled[MIXER_WIN_NR]; >>>>> }; >>>>> @@ -74,6 +77,12 @@ void exynos_hdmi_drv_attach(struct exynos_drm_hdmi_context *ctx) >>>>> hdmi_ctx = ctx; >>>>> } >>>>> >>>>> +void exynos_hdmiphy_drv_attach(struct exynos_drm_hdmi_context *ctx) >>>>> +{ >>>>> + if (ctx) >>>>> + hdmiphy_ctx = ctx; >>>>> +} >>>>> + >>>>> void exynos_mixer_drv_attach(struct exynos_drm_hdmi_context *ctx) >>>>> { >>>>> if (ctx) >>>>> @@ -88,6 +97,14 @@ void exynos_hdmi_ops_register(struct exynos_hdmi_ops *ops) >>>>> hdmi_ops = ops; >>>>> } >>>>> >>>>> +void exynos_hdmiphy_ops_register(struct exynos_hdmiphy_ops *ops) >>>>> +{ >>>>> + DRM_DEBUG_KMS("%s\n", __FILE__); >>>> >>>> Listing __FILE__ in these DRM_DEBUG_KMS traces is pretty useless. I'm >>>> sure we can determine the file from the function name. It would be >>>> slightly more useful to list __LINE__, but even more useful if you >>>> actually printed state relevant to the current function. >>>> >>> >>> OK. I will correct this. >>> >>>>> + >>>>> + if (ops) >>>>> + hdmiphy_ops = ops; >>>>> +} >>>>> + >>>>> void exynos_mixer_ops_register(struct exynos_mixer_ops *ops) >>>>> { >>>>> DRM_DEBUG_KMS("%s\n", __FILE__); >>>>> @@ -121,7 +138,7 @@ struct edid *drm_hdmi_get_edid(struct device *dev, >>>>> return NULL; >>>>> } >>>>> >>>>> -static int drm_hdmi_check_timing(struct device *dev, void *timing) >>>>> +static int drm_hdmi_check_timing(struct device *dev, void *mode) >>>>> { >>>>> struct drm_hdmi_context *ctx = to_context(dev); >>>>> int ret = 0; >>>>> @@ -129,18 +146,24 @@ static int drm_hdmi_check_timing(struct device *dev, void *timing) >>>>> DRM_DEBUG_KMS("%s\n", __FILE__); >>>>> >>>>> /* >>>>> - * Both, mixer and hdmi should be able to handle the requested mode. >>>>> - * If any of the two fails, return mode as BAD. >>>>> + * Mixer, hdmi and hdmiphy should be able to handle the requested mode. >>>>> + * If any of the them fails, return mode as BAD. >>>>> */ >>>>> >>>>> if (mixer_ops && mixer_ops->check_timing) >>>>> - ret = mixer_ops->check_timing(ctx->mixer_ctx->ctx, timing); >>>>> + ret = mixer_ops->check_timing(ctx->mixer_ctx->ctx, mode); >>>>> >>>>> if (ret) >>>>> return ret; >>>>> >>>>> if (hdmi_ops && hdmi_ops->check_timing) >>>>> - return hdmi_ops->check_timing(ctx->hdmi_ctx->ctx, timing); >>>>> + ret = hdmi_ops->check_timing(ctx->hdmi_ctx->ctx, mode); >>>>> + >>>>> + if (ret) >>>>> + return ret; >>>>> + >>>>> + if (hdmiphy_ops && hdmiphy_ops->check_timing) >>>>> + return hdmiphy_ops->check_timing(ctx->hdmiphy_ctx->ctx, mode); >>>>> >>>>> return 0; >>>>> } >>>>> @@ -254,6 +277,9 @@ static void drm_hdmi_mode_set(struct device *subdrv_dev, void *mode) >>>>> >>>>> if (hdmi_ops && hdmi_ops->mode_set) >>>>> hdmi_ops->mode_set(ctx->hdmi_ctx->ctx, mode); >>>>> + >>>>> + if (hdmiphy_ops && hdmiphy_ops->mode_set) >>>>> + hdmiphy_ops->mode_set(ctx->hdmiphy_ctx->ctx, mode); >>>>> } >>>>> >>>>> static void drm_hdmi_get_max_resol(struct device *subdrv_dev, >>>>> @@ -273,6 +299,15 @@ static void drm_hdmi_commit(struct device *subdrv_dev) >>>>> >>>>> DRM_DEBUG_KMS("%s\n", __FILE__); >>>>> >>>>> + if (hdmiphy_ops && hdmiphy_ops->prepare) >>>>> + hdmiphy_ops->prepare(ctx->hdmiphy_ctx->ctx); >>>>> + >>>>> + if (hdmi_ops && hdmi_ops->prepare) >>>>> + hdmi_ops->prepare(ctx->hdmi_ctx->ctx); >>>>> + >>>> >>>> Why are you calling these prepare functions in commit? Isn't that >>>> precisely the reason drm has a prepare callback? >>> >>> True. Currently exynos_drm_encoder_prepare is blank. I will move this to >>> prepare callback. >>> >>>> >>>>> + if (hdmiphy_ops && hdmiphy_ops->config_apply) >>>>> + hdmiphy_ops->config_apply(ctx->hdmiphy_ctx->ctx); >>>>> + >>>>> if (hdmi_ops && hdmi_ops->commit) >>>>> hdmi_ops->commit(ctx->hdmi_ctx->ctx); >>>>> } >>>>> @@ -288,6 +323,9 @@ static void drm_hdmi_dpms(struct device *subdrv_dev, int mode) >>>>> >>>>> if (hdmi_ops && hdmi_ops->dpms) >>>>> hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode); >>>>> + >>>>> + if (hdmiphy_ops && hdmiphy_ops->dpms) >>>>> + hdmiphy_ops->dpms(ctx->hdmiphy_ctx->ctx, mode); >>>> >>>> I think you have to be more careful about the ordering of things here >>>> since you'll want to power on in a different order than you power off. >>>> >>>>> } >>>>> >>>>> static void drm_hdmi_apply(struct device *subdrv_dev) >>>>> @@ -393,6 +431,11 @@ static int hdmi_subdrv_probe(struct drm_device *drm_dev, >>>>> return -EFAULT; >>>>> } >>>>> >>>>> + if (!hdmiphy_ctx) { >>>>> + DRM_ERROR("hdmiphy context not initialized.\n"); >>>>> + return -EFAULT; >>>>> + } >>>>> + >>>>> if (!mixer_ctx) { >>>>> DRM_ERROR("mixer context not initialized.\n"); >>>>> return -EFAULT; >>>>> @@ -406,6 +449,7 @@ static int hdmi_subdrv_probe(struct drm_device *drm_dev, >>>>> } >>>>> >>>>> ctx->hdmi_ctx = hdmi_ctx; >>>>> + ctx->hdmiphy_ctx = hdmiphy_ctx; >>>>> ctx->mixer_ctx = mixer_ctx; >>>>> >>>>> ctx->hdmi_ctx->drm_dev = drm_dev; >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h >>>>> index fd2ff9f..10db62e 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h >>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h >>>>> @@ -39,10 +39,19 @@ struct exynos_hdmi_ops { >>>>> void (*mode_set)(void *ctx, struct drm_display_mode *mode); >>>>> void (*get_max_resol)(void *ctx, unsigned int *width, >>>>> unsigned int *height); >>>>> + void (*prepare)(void *ctx); >>>>> void (*commit)(void *ctx); >>>>> void (*dpms)(void *ctx, int mode); >>>>> }; >>>>> >>>>> +struct exynos_hdmiphy_ops { >>>>> + int (*check_timing)(void *ctx, struct drm_display_mode *mode); >>>>> + void (*mode_set)(void *ctx, struct drm_display_mode *mode); >>>>> + void (*prepare)(void *ctx); >>>>> + int (*config_apply)(void *ctx); >>>>> + void (*dpms)(void *ctx, int mode); >>>>> +}; >>>>> + >>>>> struct exynos_mixer_ops { >>>>> /* manager */ >>>>> int (*iommu_on)(void *ctx, bool enable); >>>>> @@ -61,7 +70,9 @@ struct exynos_mixer_ops { >>>>> }; >>>>> >>>>> void exynos_hdmi_drv_attach(struct exynos_drm_hdmi_context *ctx); >>>>> +void exynos_hdmiphy_drv_attach(struct exynos_drm_hdmi_context *ctx); >>>>> void exynos_mixer_drv_attach(struct exynos_drm_hdmi_context *ctx); >>>>> void exynos_hdmi_ops_register(struct exynos_hdmi_ops *ops); >>>>> +void exynos_hdmiphy_ops_register(struct exynos_hdmiphy_ops *ops); >>>>> void exynos_mixer_ops_register(struct exynos_mixer_ops *ops); >>>>> #endif >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c >>>>> index a5ca2cc..7ee350d 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c >>>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c >>>>> @@ -194,7 +194,6 @@ struct hdmi_context { >>>>> int internal_irq; >>>>> >>>>> struct i2c_client *ddc_port; >>>>> - struct i2c_client *hdmiphy_port; >>>>> >>>>> /* current hdmiphy conf regs */ >>>>> struct hdmi_conf_regs mode_conf; >>>>> @@ -206,180 +205,6 @@ struct hdmi_context { >>>>> enum hdmi_type type; >>>>> }; >>>>> >>>>> -struct hdmiphy_config { >>>>> - int pixel_clock; >>>>> - u8 conf[32]; >>>>> -}; >>>>> - >>>>> -/* list of phy config settings */ >>>>> -static const struct hdmiphy_config hdmiphy_v13_configs[] = { >>>>> - { >>>>> - .pixel_clock = 27000000, >>>>> - .conf = { >>>>> - 0x01, 0x05, 0x00, 0xD8, 0x10, 0x1C, 0x30, 0x40, >>>>> - 0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87, >>>>> - 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, >>>>> - 0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 27027000, >>>>> - .conf = { >>>>> - 0x01, 0x05, 0x00, 0xD4, 0x10, 0x9C, 0x09, 0x64, >>>>> - 0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87, >>>>> - 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, >>>>> - 0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 74176000, >>>>> - .conf = { >>>>> - 0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xef, 0x5B, >>>>> - 0x6D, 0x10, 0x01, 0x51, 0xef, 0xF3, 0x54, 0xb9, >>>>> - 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, >>>>> - 0x22, 0x40, 0xa5, 0x26, 0x01, 0x00, 0x00, 0x00, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 74250000, >>>>> - .conf = { >>>>> - 0x01, 0x05, 0x00, 0xd8, 0x10, 0x9c, 0xf8, 0x40, >>>>> - 0x6a, 0x10, 0x01, 0x51, 0xff, 0xf1, 0x54, 0xba, >>>>> - 0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xe0, >>>>> - 0x22, 0x40, 0xa4, 0x26, 0x01, 0x00, 0x00, 0x00, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 148500000, >>>>> - .conf = { >>>>> - 0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xf8, 0x40, >>>>> - 0x6A, 0x18, 0x00, 0x51, 0xff, 0xF1, 0x54, 0xba, >>>>> - 0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xE0, >>>>> - 0x22, 0x40, 0xa4, 0x26, 0x02, 0x00, 0x00, 0x00, >>>>> - }, >>>>> - }, >>>>> -}; >>>>> - >>>>> -static const struct hdmiphy_config hdmiphy_v14_configs[] = { >>>>> - { >>>>> - .pixel_clock = 25200000, >>>>> - .conf = { >>>>> - 0x01, 0x51, 0x2A, 0x75, 0x40, 0x01, 0x00, 0x08, >>>>> - 0x82, 0x80, 0xfc, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0xf4, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 27000000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x22, 0x51, 0x40, 0x08, 0xfc, 0x20, >>>>> - 0x98, 0xa0, 0xcb, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x06, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0xe4, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 27027000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x2d, 0x72, 0x40, 0x64, 0x12, 0x08, >>>>> - 0x43, 0xa0, 0x0e, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0xe3, 0x24, 0x00, 0x00, 0x00, 0x01, 0x00, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 36000000, >>>>> - .conf = { >>>>> - 0x01, 0x51, 0x2d, 0x55, 0x40, 0x01, 0x00, 0x08, >>>>> - 0x82, 0x80, 0x0e, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0xab, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 40000000, >>>>> - .conf = { >>>>> - 0x01, 0x51, 0x32, 0x55, 0x40, 0x01, 0x00, 0x08, >>>>> - 0x82, 0x80, 0x2c, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0x9a, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 65000000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x36, 0x34, 0x40, 0x1e, 0x0a, 0x08, >>>>> - 0x82, 0xa0, 0x45, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0xbd, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 74176000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x3e, 0x35, 0x40, 0x5b, 0xde, 0x08, >>>>> - 0x82, 0xa0, 0x73, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x56, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0xa6, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 74250000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x1f, 0x10, 0x40, 0x40, 0xf8, 0x08, >>>>> - 0x81, 0xa0, 0xba, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x3c, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0xa5, 0x24, 0x01, 0x00, 0x00, 0x01, 0x00, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 83500000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x23, 0x11, 0x40, 0x0c, 0xfb, 0x08, >>>>> - 0x85, 0xa0, 0xd1, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0x93, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 106500000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x2c, 0x12, 0x40, 0x0c, 0x09, 0x08, >>>>> - 0x84, 0xa0, 0x0a, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0x73, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 108000000, >>>>> - .conf = { >>>>> - 0x01, 0x51, 0x2d, 0x15, 0x40, 0x01, 0x00, 0x08, >>>>> - 0x82, 0x80, 0x0e, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0xc7, 0x25, 0x03, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 146250000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x3d, 0x15, 0x40, 0x18, 0xfd, 0x08, >>>>> - 0x83, 0xa0, 0x6e, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0x50, 0x25, 0x03, 0x00, 0x00, 0x01, 0x80, >>>>> - }, >>>>> - }, >>>>> - { >>>>> - .pixel_clock = 148500000, >>>>> - .conf = { >>>>> - 0x01, 0xd1, 0x1f, 0x00, 0x40, 0x40, 0xf8, 0x08, >>>>> - 0x81, 0xa0, 0xba, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> - 0x3c, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> - 0x54, 0x4b, 0x25, 0x03, 0x00, 0x00, 0x01, 0x00, >>>>> - }, >>>>> - }, >>>>> -}; >>>>> - >>>>> struct hdmi_infoframe { >>>>> enum HDMI_PACKET_TYPE type; >>>>> u8 ver; >>>>> @@ -774,46 +599,6 @@ static struct edid *hdmi_get_edid(void *ctx, struct drm_connector *connector) >>>>> return raw_edid; >>>>> } >>>>> >>>>> -static int hdmi_find_phy_conf(struct hdmi_context *hdata, u32 pixel_clock) >>>>> -{ >>>>> - const struct hdmiphy_config *confs; >>>>> - int count, i; >>>>> - >>>>> - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> - >>>>> - if (hdata->type == HDMI_TYPE13) { >>>>> - confs = hdmiphy_v13_configs; >>>>> - count = ARRAY_SIZE(hdmiphy_v13_configs); >>>>> - } else if (hdata->type == HDMI_TYPE14) { >>>>> - confs = hdmiphy_v14_configs; >>>>> - count = ARRAY_SIZE(hdmiphy_v14_configs); >>>>> - } else >>>>> - return -EINVAL; >>>>> - >>>>> - for (i = 0; i < count; i++) >>>>> - if (confs[i].pixel_clock == pixel_clock) >>>>> - return i; >>>>> - >>>>> - DRM_DEBUG_KMS("Could not find phy config for %d\n", pixel_clock); >>>>> - return -EINVAL; >>>>> -} >>>>> - >>>>> -static int hdmi_check_timing(void *ctx, struct drm_display_mode *mode) >>>>> -{ >>>>> - struct hdmi_context *hdata = ctx; >>>>> - int ret; >>>>> - >>>>> - DRM_DEBUG_KMS("%s : xres=%d, yres=%d, refresh=%d, intl=%d clock=%d\n", >>>>> - __func__, mode->hdisplay, mode->vdisplay, >>>>> - mode->vrefresh, (mode->flags & DRM_MODE_FLAG_INTERLACE) ? true : >>>>> - false, mode->clock * 1000); >>>>> - >>>>> - ret = hdmi_find_phy_conf(hdata, mode->clock * 1000); >>>>> - if (ret < 0) >>>>> - return ret; >>>>> - return 0; >>>>> -} >>>>> - >>>>> static void hdmi_set_acr(u32 freq, u8 *acr) >>>>> { >>>>> u32 n, cts; >>>>> @@ -1309,20 +1094,12 @@ static void hdmi_timing_apply(struct hdmi_context *hdata) >>>>> >>>>> static void hdmiphy_conf_reset(struct hdmi_context *hdata) >>>>> { >>>>> - u8 buffer[2]; >>>>> u32 reg; >>>>> >>>>> clk_disable(hdata->res.sclk_hdmi); >>>>> clk_set_parent(hdata->res.sclk_hdmi, hdata->res.sclk_pixel); >>>>> clk_enable(hdata->res.sclk_hdmi); >>>>> >>>>> - /* operation mode */ >>>>> - buffer[0] = 0x1f; >>>>> - buffer[1] = 0x00; >>>>> - >>>>> - if (hdata->hdmiphy_port) >>>>> - i2c_master_send(hdata->hdmiphy_port, buffer, 2); >>>>> - >>>>> if (hdata->type == HDMI_TYPE13) >>>>> reg = HDMI_V13_PHY_RSTOUT; >>>>> else >>>>> @@ -1335,102 +1112,6 @@ static void hdmiphy_conf_reset(struct hdmi_context *hdata) >>>>> usleep_range(10000, 12000); >>>>> } >>>>> >>>>> -static void hdmiphy_poweron(struct hdmi_context *hdata) >>>>> -{ >>>>> - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> - >>>>> - if (hdata->type == HDMI_TYPE14) >>>>> - hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, 0, >>>>> - HDMI_PHY_POWER_OFF_EN); >>>>> -} >>>>> - >>>>> -static void hdmiphy_poweroff(struct hdmi_context *hdata) >>>>> -{ >>>>> - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> - >>>>> - if (hdata->type == HDMI_TYPE14) >>>>> - hdmi_reg_writemask(hdata, HDMI_PHY_CON_0, ~0, >>>>> - HDMI_PHY_POWER_OFF_EN); >>>> >>>> I assume these register writes are replaced with i2c operations in the >>>> new phy driver? >>> >>> It is done for exynos5 phy. I am not having the sequence for exynos4. I >>> will add it later. >>> >>>> >>>>> -} >>>>> - >>>>> -static void hdmiphy_conf_apply(struct hdmi_context *hdata) >>>>> -{ >>>>> - const u8 *hdmiphy_data; >>>>> - u8 buffer[32]; >>>>> - u8 operation[2]; >>>>> - u8 read_buffer[32] = {0, }; >>>>> - int ret; >>>>> - int i; >>>>> - >>>>> - if (!hdata->hdmiphy_port) { >>>>> - DRM_ERROR("hdmiphy is not attached\n"); >>>>> - return; >>>>> - } >>>>> - >>>>> - /* pixel clock */ >>>>> - i = hdmi_find_phy_conf(hdata, hdata->mode_conf.pixel_clock); >>>>> - if (i < 0) { >>>>> - DRM_ERROR("failed to find hdmiphy conf\n"); >>>>> - return; >>>>> - } >>>>> - >>>>> - if (hdata->type == HDMI_TYPE13) { >>>>> - hdmiphy_data = hdmiphy_v13_configs[i].conf; >>>>> - } else { >>>>> - hdmiphy_data = hdmiphy_v14_configs[i].conf; >>>>> - } >>>>> - >>>>> - memcpy(buffer, hdmiphy_data, 32); >>>>> - ret = i2c_master_send(hdata->hdmiphy_port, buffer, 32); >>>>> - if (ret != 32) { >>>>> - DRM_ERROR("failed to configure HDMIPHY via I2C\n"); >>>>> - return; >>>>> - } >>>>> - >>>>> - usleep_range(10000, 12000); >>>>> - >>>>> - /* operation mode */ >>>>> - operation[0] = 0x1f; >>>>> - operation[1] = 0x80; >>>>> - >>>>> - ret = i2c_master_send(hdata->hdmiphy_port, operation, 2); >>>>> - if (ret != 2) { >>>>> - DRM_ERROR("failed to enable hdmiphy\n"); >>>>> - return; >>>>> - } >>>>> - >>>>> - ret = i2c_master_recv(hdata->hdmiphy_port, read_buffer, 32); >>>>> - if (ret < 0) { >>>>> - DRM_ERROR("failed to read hdmiphy config\n"); >>>>> - return; >>>>> - } >>>>> - >>>>> - for (i = 0; i < ret; i++) >>>>> - DRM_DEBUG_KMS("hdmiphy[0x%02x] write[0x%02x] - " >>>>> - "recv [0x%02x]\n", i, buffer[i], read_buffer[i]); >>>>> -} >>>>> - >>>>> -static void hdmi_conf_apply(struct hdmi_context *hdata) >>>>> -{ >>>>> - DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> - >>>>> - hdmiphy_conf_reset(hdata); >>>>> - hdmiphy_conf_apply(hdata); >>>>> - >>>>> - mutex_lock(&hdata->hdmi_mutex); >>>>> - hdmi_conf_reset(hdata); >>>>> - hdmi_conf_init(hdata); >>>>> - mutex_unlock(&hdata->hdmi_mutex); >>>>> - >>>>> - hdmi_audio_init(hdata); >>>>> - >>>>> - /* setting core registers */ >>>>> - hdmi_timing_apply(hdata); >>>>> - hdmi_audio_control(hdata, true); >>>>> - >>>>> - hdmi_regs_dump(hdata, "start"); >>>>> -} >>>>> - >>>>> static void hdmi_set_reg(u8 *reg_pair, int num_bytes, u32 value) >>>>> { >>>>> int i; >>>>> @@ -1448,7 +1129,6 @@ static void hdmi_v13_mode_set(struct hdmi_context *hdata, >>>>> >>>>> hdata->mode_conf.cea_video_id = >>>>> drm_match_cea_mode((struct drm_display_mode *)m); >>>>> - hdata->mode_conf.pixel_clock = m->clock * 1000; >>>> >>>> I don't think you use pixel_clock in mode_conf any longer, please >>>> remove it from the struct. >>>> >>> >>> Correct. I will remove pixel_clock from struct hdmi_conf_regs. >>> >>>>> >>>>> hdmi_set_reg(core->h_blank, 2, m->htotal - m->hdisplay); >>>>> hdmi_set_reg(core->h_v_line, 3, (m->htotal << 12) | m->vtotal); >>>>> @@ -1544,7 +1224,6 @@ static void hdmi_v14_mode_set(struct hdmi_context *hdata, >>>>> >>>>> hdata->mode_conf.cea_video_id = >>>>> drm_match_cea_mode((struct drm_display_mode *)m); >>>>> - hdata->mode_conf.pixel_clock = m->clock * 1000; >>>>> >>>>> hdmi_set_reg(core->h_blank, 2, m->htotal - m->hdisplay); >>>>> hdmi_set_reg(core->v_line, 2, m->vtotal); >>>>> @@ -1670,13 +1349,32 @@ static void hdmi_get_max_resol(void *ctx, unsigned int *width, >>>>> *height = MAX_HEIGHT; >>>>> } >>>>> >>>>> +static void hdmi_commit_prepare(void *ctx) >>>> >>>> I don't know why the "_commit" is there, just hdmi_prepare will be fine. >>>> >>> Ok. >>> >>>>> +{ >>>>> + struct hdmi_context *hdata = ctx; >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + hdmiphy_conf_reset(hdata); >>>> >>>> There's still phy-specific kruft laying around the driver. There are >>>> now a bunch of implicit dependencies between the hdmi driver and the >>>> hdmiphy driver. >>>> >>> >>> I need to follow this sequence: >>> >>> phy off (hdmiphy_prepare) -> phy reset using HDMI IP REG (hdmi_prepare) -> >>> phy config (hdmiphy_config_apply) -> hdmi ip config (hdmi_commit) >>> >>> Phy reset needs to be done by toggling Phy_reset bit in HDMI IP register which >>> cannot be accessed in hdmiphy driver. I didn't see any better way. >>> >>>> Another example is the sclk_hdmiphy clock, which is still controlled >>>> by the hdmi driver, it should be controlled by the phy. >>>> >>> >>> sclk_hdmiphy is one of the source clock for hdmi IP. HDMI IP needs to change the >>> source between sclk_hdmiphy (only if stable) else pixel clock. Above >>> code looks ok >>> to me. Please explain a bit mode on this. >>> >>>>> +} >>>>> + >>>>> static void hdmi_commit(void *ctx) >>>>> { >>>>> struct hdmi_context *hdata = ctx; >>>>> >>>>> DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> >>>>> - hdmi_conf_apply(hdata); >>>>> + mutex_lock(&hdata->hdmi_mutex); >>>>> + hdmi_conf_reset(hdata); >>>>> + hdmi_conf_init(hdata); >>>>> + mutex_unlock(&hdata->hdmi_mutex); >>>>> + >>>>> + hdmi_audio_init(hdata); >>>>> + >>>>> + /* setting core registers */ >>>>> + hdmi_timing_apply(hdata); >>>>> + hdmi_audio_control(hdata, true); >>>>> + >>>>> + hdmi_regs_dump(hdata, "start"); >>>>> } >>>>> >>>>> static void hdmi_poweron(struct hdmi_context *hdata) >>>>> @@ -1699,8 +1397,6 @@ static void hdmi_poweron(struct hdmi_context *hdata) >>>>> clk_enable(res->hdmiphy); >>>>> clk_enable(res->hdmi); >>>>> clk_enable(res->sclk_hdmi); >>>>> - >>>>> - hdmiphy_poweron(hdata); >>>>> } >>>>> >>>>> static void hdmi_poweroff(struct hdmi_context *hdata) >>>>> @@ -1719,7 +1415,6 @@ static void hdmi_poweroff(struct hdmi_context *hdata) >>>>> * its reset state seems to meet the condition. >>>>> */ >>>>> hdmiphy_conf_reset(hdata); >>>>> - hdmiphy_poweroff(hdata); >>>>> >>>>> clk_disable(res->sclk_hdmi); >>>>> clk_disable(res->hdmi); >>>>> @@ -1761,11 +1456,11 @@ static struct exynos_hdmi_ops hdmi_ops = { >>>>> /* display */ >>>>> .is_connected = hdmi_is_connected, >>>>> .get_edid = hdmi_get_edid, >>>>> - .check_timing = hdmi_check_timing, >>>>> >>>>> /* manager */ >>>>> .mode_set = hdmi_mode_set, >>>>> .get_max_resol = hdmi_get_max_resol, >>>>> + .prepare = hdmi_commit_prepare, >>>>> .commit = hdmi_commit, >>>>> .dpms = hdmi_dpms, >>>>> }; >>>>> @@ -1878,7 +1573,7 @@ fail: >>>>> return -ENODEV; >>>>> } >>>>> >>>>> -static struct i2c_client *hdmi_ddc, *hdmi_hdmiphy; >>>>> +static struct i2c_client *hdmi_ddc; >>>>> >>>>> void hdmi_attach_ddc_client(struct i2c_client *ddc) >>>>> { >>>>> @@ -1886,12 +1581,6 @@ void hdmi_attach_ddc_client(struct i2c_client *ddc) >>>>> hdmi_ddc = ddc; >>>>> } >>>>> >>>>> -void hdmi_attach_hdmiphy_client(struct i2c_client *hdmiphy) >>>>> -{ >>>>> - if (hdmiphy) >>>>> - hdmi_hdmiphy = hdmiphy; >>>>> -} >>>>> - >>>>> #ifdef CONFIG_OF >>>>> static struct s5p_hdmi_platform_data *drm_hdmi_dt_parse_pdata >>>>> (struct device *dev) >>>>> @@ -2051,27 +1740,18 @@ static int hdmi_probe(struct platform_device *pdev) >>>>> >>>>> hdata->ddc_port = hdmi_ddc; >>>>> >>>>> - /* hdmiphy i2c driver */ >>>>> - if (i2c_add_driver(&hdmiphy_driver)) { >>>>> - DRM_ERROR("failed to register hdmiphy i2c driver\n"); >>>>> - ret = -ENOENT; >>>>> - goto err_ddc; >>>>> - } >>>>> - >>>>> - hdata->hdmiphy_port = hdmi_hdmiphy; >>>>> - >>>>> hdata->external_irq = gpio_to_irq(hdata->hpd_gpio); >>>>> if (hdata->external_irq < 0) { >>>>> DRM_ERROR("failed to get GPIO external irq\n"); >>>>> ret = hdata->external_irq; >>>>> - goto err_hdmiphy; >>>>> + goto err_ddc; >>>>> } >>>>> >>>>> hdata->internal_irq = platform_get_irq(pdev, 0); >>>>> if (hdata->internal_irq < 0) { >>>>> DRM_ERROR("failed to get platform internal irq\n"); >>>>> ret = hdata->internal_irq; >>>>> - goto err_hdmiphy; >>>>> + goto err_ddc; >>>>> } >>>>> >>>>> hdata->hpd = gpio_get_value(hdata->hpd_gpio); >>>>> @@ -2082,7 +1762,7 @@ static int hdmi_probe(struct platform_device *pdev) >>>>> "hdmi_external", drm_hdmi_ctx); >>>>> if (ret) { >>>>> DRM_ERROR("failed to register hdmi external interrupt\n"); >>>>> - goto err_hdmiphy; >>>>> + goto err_ddc; >>>>> } >>>>> >>>>> ret = request_threaded_irq(hdata->internal_irq, NULL, >>>>> @@ -2105,8 +1785,6 @@ static int hdmi_probe(struct platform_device *pdev) >>>>> >>>>> err_free_irq: >>>>> free_irq(hdata->external_irq, drm_hdmi_ctx); >>>>> -err_hdmiphy: >>>>> - i2c_del_driver(&hdmiphy_driver); >>>>> err_ddc: >>>>> i2c_del_driver(&ddc_driver); >>>>> return ret; >>>>> @@ -2125,9 +1803,6 @@ static int hdmi_remove(struct platform_device *pdev) >>>>> free_irq(hdata->internal_irq, hdata); >>>>> free_irq(hdata->external_irq, hdata); >>>>> >>>>> - >>>>> - /* hdmiphy i2c driver */ >>>>> - i2c_del_driver(&hdmiphy_driver); >>>>> /* DDC i2c driver */ >>>>> i2c_del_driver(&ddc_driver); >>>>> >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.h b/drivers/gpu/drm/exynos/exynos_hdmi.h >>>>> index 0ddf395..6595d7b 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.h >>>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.h >>>>> @@ -15,7 +15,6 @@ >>>>> #define _EXYNOS_HDMI_H_ >>>>> >>>>> void hdmi_attach_ddc_client(struct i2c_client *ddc); >>>>> -void hdmi_attach_hdmiphy_client(struct i2c_client *hdmiphy); >>>>> >>>>> extern struct i2c_driver hdmiphy_driver; >>>>> extern struct i2c_driver ddc_driver; >>>>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmiphy.c b/drivers/gpu/drm/exynos/exynos_hdmiphy.c >>>>> index ea49d13..0d47302 100644 >>>>> --- a/drivers/gpu/drm/exynos/exynos_hdmiphy.c >>>>> +++ b/drivers/gpu/drm/exynos/exynos_hdmiphy.c >>>>> @@ -16,33 +16,437 @@ >>>>> #include <linux/kernel.h> >>>>> #include <linux/i2c.h> >>>>> #include <linux/module.h> >>>>> +#include <linux/pm_runtime.h> >>>>> +#include <linux/clk.h> >>>>> >>>>> #include "exynos_drm_drv.h" >>>>> +#include "exynos_drm_hdmi.h" >>>>> #include "exynos_hdmi.h" >>>>> >>>>> +#include "regs-hdmiphy.h" >>>>> >>>>> -static int hdmiphy_probe(struct i2c_client *client, >>>>> - const struct i2c_device_id *id) >>>>> +#define HDMIPHY_REG_COUNT (32) >>>> >>>> No need to include parentheses. >>> >>> Ok. >>>> >>>>> + >>>>> +enum hdmiphy_type { >>>>> + HDMIPHY_EXYNOS4210, >>>>> + HDMIPHY_EXYNOS4212, >>>>> +}; >>>>> + >>>>> +struct hdmiphy_context { >>>>> + struct device *dev; >>>>> + bool powered; >>>>> + void *parent_ctx; >>>>> + enum hdmiphy_type type; >>>>> + const struct hdmiphy_config *conf; >>>>> + struct clk *hdmiphy; >>>>> +}; >>>>> + >>>>> +struct hdmiphy_config { >>>>> + int pixel_clock; >>>>> + u8 conf[HDMIPHY_REG_COUNT]; >>>>> +}; >>>>> + >>>>> +/* list of all required phy config settings */ >>>>> +static const struct hdmiphy_config hdmiphy_4212_configs[] = { >>>>> + { >>>>> + .pixel_clock = 25200000, >>>>> + .conf = { >>>>> + 0x01, 0x51, 0x2A, 0x75, 0x40, 0x01, 0x00, 0x08, >>>>> + 0x82, 0x80, 0xfc, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0xf4, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 27000000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x22, 0x51, 0x40, 0x08, 0xfc, 0x20, >>>>> + 0x98, 0xa0, 0xcb, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x06, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0xe4, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 27027000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x2d, 0x72, 0x40, 0x64, 0x12, 0x08, >>>>> + 0x43, 0xa0, 0x0e, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0xe3, 0x24, 0x00, 0x00, 0x00, 0x01, 0x00, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 36000000, >>>>> + .conf = { >>>>> + 0x01, 0x51, 0x2d, 0x55, 0x40, 0x01, 0x00, 0x08, >>>>> + 0x82, 0x80, 0x0e, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0xab, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 40000000, >>>>> + .conf = { >>>>> + 0x01, 0x51, 0x32, 0x55, 0x40, 0x01, 0x00, 0x08, >>>>> + 0x82, 0x80, 0x2c, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0x9a, 0x24, 0x00, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 65000000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x36, 0x34, 0x40, 0x1e, 0x0a, 0x08, >>>>> + 0x82, 0xa0, 0x45, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0xbd, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 74176000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x3e, 0x35, 0x40, 0x5b, 0xde, 0x08, >>>>> + 0x82, 0xa0, 0x73, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x56, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0xa6, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 74250000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x1f, 0x10, 0x40, 0x40, 0xf8, 0x08, >>>>> + 0x81, 0xa0, 0xba, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x3c, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0xa5, 0x24, 0x01, 0x00, 0x00, 0x01, 0x00, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 83500000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x23, 0x11, 0x40, 0x0c, 0xfb, 0x08, >>>>> + 0x85, 0xa0, 0xd1, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0x93, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 106500000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x2c, 0x12, 0x40, 0x0c, 0x09, 0x08, >>>>> + 0x84, 0xa0, 0x0a, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0x73, 0x24, 0x01, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 108000000, >>>>> + .conf = { >>>>> + 0x01, 0x51, 0x2d, 0x15, 0x40, 0x01, 0x00, 0x08, >>>>> + 0x82, 0x80, 0x0e, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0xc7, 0x25, 0x03, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 146250000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x3d, 0x15, 0x40, 0x18, 0xfd, 0x08, >>>>> + 0x83, 0xa0, 0x6e, 0xd9, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x08, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0x50, 0x25, 0x03, 0x00, 0x00, 0x01, 0x80, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 148500000, >>>>> + .conf = { >>>>> + 0x01, 0xd1, 0x1f, 0x00, 0x40, 0x40, 0xf8, 0x08, >>>>> + 0x81, 0xa0, 0xba, 0xd8, 0x45, 0xa0, 0xac, 0x80, >>>>> + 0x3c, 0x80, 0x11, 0x04, 0x02, 0x22, 0x44, 0x86, >>>>> + 0x54, 0x4b, 0x25, 0x03, 0x00, 0x00, 0x01, 0x00, >>>>> + }, >>>>> + }, >>>>> +}; >>>>> + >>>>> +static const struct hdmiphy_config hdmiphy_4210_configs[] = { >>>>> + { >>>>> + .pixel_clock = 27000000, >>>>> + .conf = { >>>>> + 0x01, 0x05, 0x00, 0xD8, 0x10, 0x1C, 0x30, 0x40, >>>>> + 0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87, >>>>> + 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, >>>>> + 0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 27027000, >>>>> + .conf = { >>>>> + 0x01, 0x05, 0x00, 0xD4, 0x10, 0x9C, 0x09, 0x64, >>>>> + 0x6B, 0x10, 0x02, 0x51, 0xDF, 0xF2, 0x54, 0x87, >>>>> + 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, >>>>> + 0x22, 0x40, 0xE3, 0x26, 0x00, 0x00, 0x00, 0x00, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 74176000, >>>>> + .conf = { >>>>> + 0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xef, 0x5B, >>>>> + 0x6D, 0x10, 0x01, 0x51, 0xef, 0xF3, 0x54, 0xb9, >>>>> + 0x84, 0x00, 0x30, 0x38, 0x00, 0x08, 0x10, 0xE0, >>>>> + 0x22, 0x40, 0xa5, 0x26, 0x01, 0x00, 0x00, 0x00, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 74250000, >>>>> + .conf = { >>>>> + 0x01, 0x05, 0x00, 0xd8, 0x10, 0x9c, 0xf8, 0x40, >>>>> + 0x6a, 0x10, 0x01, 0x51, 0xff, 0xf1, 0x54, 0xba, >>>>> + 0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xe0, >>>>> + 0x22, 0x40, 0xa4, 0x26, 0x01, 0x00, 0x00, 0x00, >>>>> + }, >>>>> + }, >>>>> + { >>>>> + .pixel_clock = 148500000, >>>>> + .conf = { >>>>> + 0x01, 0x05, 0x00, 0xD8, 0x10, 0x9C, 0xf8, 0x40, >>>>> + 0x6A, 0x18, 0x00, 0x51, 0xff, 0xF1, 0x54, 0xba, >>>>> + 0x84, 0x00, 0x10, 0x38, 0x00, 0x08, 0x10, 0xE0, >>>>> + 0x22, 0x40, 0xa4, 0x26, 0x02, 0x00, 0x00, 0x00, >>>>> + }, >>>>> + }, >>>>> +}; >>>>> + >>>>> +static const struct hdmiphy_config *hdmiphy_find_conf(void *ctx, >>>>> + const struct drm_display_mode *mode) >>>>> +{ >>>>> + struct hdmiphy_context *hdata = ctx; >>>>> + const struct hdmiphy_config *confs; >>>>> + int count, i; >>>>> + >>>>> + switch (hdata->type) { >>>>> + case HDMIPHY_EXYNOS4212: >>>>> + confs = hdmiphy_4212_configs; >>>>> + count = ARRAY_SIZE(hdmiphy_4212_configs); >>>>> + break; >>>>> + case HDMIPHY_EXYNOS4210: >>>>> + confs = hdmiphy_4210_configs; >>>>> + count = ARRAY_SIZE(hdmiphy_4210_configs); >>>>> + break; >>>>> + default: >>>>> + return NULL; >>>> >>>> Should probably DRM_ERROR here. >>>> >>> ok. I will add that. >>> >>>>> + } >>>>> + >>>>> + for (i = 0; i < count; i++) >>>>> + if (confs[i].pixel_clock == (mode->clock * 1000)) >>>>> + return &confs[i]; >>>>> + >>>>> + return NULL; >>>>> +} >>>>> + >>>>> +static int hdmiphy_check_timing(void *ctx, struct drm_display_mode *mode) >>>>> { >>>>> - hdmi_attach_hdmiphy_client(client); >>>>> + const struct hdmiphy_config *conf; >>>>> + DRM_DEBUG_KMS("%s : xres=%d, yres=%d, refresh=%d, intl=%d clock=%d\n", >>>>> + __func__, mode->hdisplay, mode->vdisplay, >>>>> + mode->vrefresh, (mode->flags & DRM_MODE_FLAG_INTERLACE) >>>>> + ? true : false, mode->clock * 1000); >>>> >>>> No need to print __func__ here, DRM_DEBUG_KMS will do that for you. I >>>> think you should also parse the flags to a string instead of a bool >>>> cast to an int. >>>> >>> ok. >>> >>>>> >>>>> - dev_info(&client->adapter->dev, "attached s5p_hdmiphy " >>>>> - "into i2c adapter successfully\n"); >>>>> + conf = hdmiphy_find_conf(ctx, mode); >>>>> + if (!conf) { >>>>> + DRM_DEBUG_KMS("Display Mode is not supported.\n"); >>>>> + return -EINVAL; >>>>> + } >>>>> >>>>> return 0; >>>>> } >>>>> >>>>> -static int hdmiphy_remove(struct i2c_client *client) >>>>> +static void hdmiphy_mode_set(void *ctx, struct drm_display_mode *mode) >>>>> +{ >>>>> + struct hdmiphy_context *hdata = ctx; >>>>> + >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>> >>>> No need to print __func__ here, DRM_DEBUG_KMS will do that for you. >>>> This comment stand for the rest of the calls, as well. >>>> >>>>> + >>>>> + hdata->conf = hdmiphy_find_conf(ctx, mode); >>>>> +} >>>>> + >>>>> +static void hdmiphy_config_prepare(void *ctx) >>>>> +{ >>>>> + struct hdmiphy_context *hdata = ctx; >>>>> + const struct i2c_client *client = to_i2c_client(hdata->dev); >>>>> + u8 buffer[2]; >>>>> + >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + /* operation mode */ >>>>> + buffer[0] = HDMIPHY_MODE_SET_DONE; >>>>> + buffer[1] = 0x00; >>>>> + >>>>> + if (client) >>>>> + i2c_master_send(client, buffer, 2); >>>>> +} >>>>> + >>>>> +static int hdmiphy_config_apply(void *ctx) >>>>> { >>>>> - dev_info(&client->adapter->dev, "detached s5p_hdmiphy " >>>>> - "from i2c adapter successfully\n"); >>>>> + struct hdmiphy_context *hdata = ctx; >>>>> + struct i2c_client *client = to_i2c_client(hdata->dev); >>>>> + const u8 *hdmiphy_data; >>>>> + u8 buffer[HDMIPHY_REG_COUNT]; >>>>> + u8 operation[2]; >>>>> + u8 read_buffer[HDMIPHY_REG_COUNT] = {0, }; >>>>> + int ret; >>>>> + int i; >>>>> + >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + /* pixel clock */ >>>>> + if (hdata->conf && client) >>>>> + hdmiphy_data = hdata->conf->conf; >>>>> + else >>>>> + return -EINVAL; >>>>> + >>>>> + memcpy(buffer, hdmiphy_data, HDMIPHY_REG_COUNT); >>>>> + >>>>> + ret = i2c_master_send(client, buffer, HDMIPHY_REG_COUNT); >>>>> + if (ret != HDMIPHY_REG_COUNT) { >>>>> + DRM_ERROR("failed to configure HDMIPHY via I2C\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + usleep_range(10000, 12000); >>>>> + >>>>> + /* operation mode */ >>>>> + operation[0] = HDMIPHY_MODE_SET_DONE; >>>>> + operation[1] = HDMIPHY_MODE_EN; >>>>> + >>>>> + ret = i2c_master_send(client, operation, 2); >>>>> + if (ret != 2) { >>>>> + DRM_ERROR("failed to enable hdmiphy\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + ret = i2c_master_recv(client, read_buffer, HDMIPHY_REG_COUNT); >>>>> + if (ret < 0) { >>>>> + DRM_ERROR("failed to read hdmiphy config\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + for (i = 0; i < ret; i++) >>>>> + DRM_DEBUG_KMS("hdmiphy[0x%02x] wr[0x%02x], rd[0x%02x]\n", >>>>> + i, buffer[i], read_buffer[i]); >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static void hdmiphy_dpms(void *ctx, int mode) >>>>> +{ >>>>> + struct hdmiphy_context *hdata = ctx; >>>>> + >>>>> + DRM_DEBUG_KMS("[%d] %s mode %d\n", __LINE__, __func__, mode); >>>>> + >>>>> + switch (mode) { >>>>> + case DRM_MODE_DPMS_ON: >>>>> + if (pm_runtime_suspended(hdata->dev)) >>>>> + pm_runtime_get_sync(hdata->dev); >>>>> + break; >>>>> + case DRM_MODE_DPMS_STANDBY: >>>>> + case DRM_MODE_DPMS_SUSPEND: >>>>> + case DRM_MODE_DPMS_OFF: >>>>> + if (!pm_runtime_suspended(hdata->dev)) >>>>> + pm_runtime_put_sync(hdata->dev); >>>>> + break; >>>>> + default: >>>>> + DRM_DEBUG_KMS("unknown dpms mode: %d\n", mode); >>>>> + break; >>>>> + } >>>>> +} >>>>> + >>>>> +static int hdmiphy_update_bits(struct i2c_client *client, u8 *reg_cache, >>>>> + u8 reg, u8 mask, u8 val) >>>>> +{ >>>>> + int ret; >>>>> + u8 buffer[2]; >>>>> + >>>>> + buffer[0] = reg; >>>>> + buffer[1] = (reg_cache[reg] & ~mask) | (val & mask); >>>>> + reg_cache[reg] = buffer[1]; >>>>> + >>>>> + ret = i2c_master_send(client, buffer, 2); >>>>> + if (ret != 2) >>>>> + return -EIO; >>>>> >>>>> return 0; >>>>> } >>>>> >>>>> +static int hdmiphy_4412_turn_on(struct i2c_client *client, bool on) >>>> >>>> This name is misleading. Please change it to something that doesn't >>>> convey a power state, such as hdmiphy_4412_power >>>> >>> ok. I will change that. >>> >>>>> +{ >>>>> + u8 reg_cache[HDMIPHY_REG_COUNT] = { 0 }; >>>>> + u8 buffer[2]; >>>>> + int ret; >>>>> + >>>>> + DRM_DEBUG_KMS("%s: hdmiphy is %s\n", __func__, on ? "on" : "off"); >>>>> + >>>>> + /* Cache all hdmi-phy registers to make the code below faster */ >>>>> + buffer[0] = 0x0; >>>>> + ret = i2c_master_send(client, buffer, 1); >>>>> + if (ret != 1) { >>>>> + ret = -EIO; >>>>> + goto exit; >>>>> + } >>>>> + ret = i2c_master_recv(client, reg_cache, HDMIPHY_REG_COUNT); >>>>> + if (ret != HDMIPHY_REG_COUNT) { >>>>> + ret = -EIO; >>>>> + goto exit; >>>>> + } >>>>> + >>>>> + /* Change to/from configuration from/to operation mode */ >>>>> + ret = hdmiphy_update_bits(client, reg_cache, HDMIPHY_MODE_SET_DONE, >>>>> + HDMIPHY_MODE_EN, on ? ~0 : 0); >>>>> + if (ret) >>>>> + goto exit; >>>>> + >>>>> + /* >>>>> + * Turn off "oscpad" if !on; it turns on again in >>>>> + * during phy-configuration. >>>>> + */ >>>>> + if (!on) >>>>> + ret = hdmiphy_update_bits(client, reg_cache, >>>>> + HDMIPHY_4212_OSC_PAD_CON, HDMIPHY_OSC_PAD_EN, 0); >>>>> + if (ret) >>>>> + goto exit; >>>>> + >>>>> + /* Disable powerdown if on; enable if !on */ >>>> >>>> This comment really isn't useful, it's just describing the code. >>> >>> I will remove this comment. >>> >>>> >>>>> + ret = hdmiphy_update_bits(client, reg_cache, HDMIPHY_4212_PD_CON, >>>>> + HDMIPHY_PDEN, on ? 0 : ~0); >>>>> + if (ret) >>>>> + goto exit; >>>>> + ret = hdmiphy_update_bits(client, reg_cache, HDMIPHY_4212_PD_CON, >>>>> + HDMIPHY_PD_ALL, on ? 0 : ~0); >>>>> + if (ret) >>>>> + goto exit; >>>>> + >>>>> + /* Disable pixel clock generator block if !on */ >>>>> + if (!on) >>>>> + ret = hdmiphy_update_bits(client, reg_cache, >>>>> + HDMIPHY_4212_PCG_CON, HDMIPHY_PCG_RESET_EN, 0); >>>>> + if (ret) >>>>> + goto exit; >>>>> + >>>>> +exit: >>>>> + /* Don't expect any errors so just do a single warn */ >>>>> + WARN_ON(ret); >>>>> + >>>>> + return ret; >>>>> +} >>>>> + >>>>> +static struct exynos_hdmiphy_ops hdmiphy_ops = { >>>>> + .check_timing = hdmiphy_check_timing, >>>>> + .mode_set = hdmiphy_mode_set, >>>>> + .prepare = hdmiphy_config_prepare, >>>>> + .config_apply = hdmiphy_config_apply, >>>>> + .dpms = hdmiphy_dpms, >>>>> +}; >>>>> + >>>>> static const struct i2c_device_id hdmiphy_id[] = { >>>>> - { "s5p_hdmiphy", 0 }, >>>>> - { "exynos5-hdmiphy", 0 }, >>>>> + { "s5p_hdmiphy", HDMIPHY_EXYNOS4210 }, >>>>> + { "exynos5-hdmiphy", HDMIPHY_EXYNOS4212 }, >>>>> { }, >>>>> }; >>>>> >>>>> @@ -50,21 +454,183 @@ static const struct i2c_device_id hdmiphy_id[] = { >>>>> static struct of_device_id hdmiphy_match_types[] = { >>>>> { >>>>> .compatible = "samsung,exynos5-hdmiphy", >>>>> + .data = (void *)HDMIPHY_EXYNOS4212, >>>>> }, { >>>>> /* end node */ >>>>> } >>>>> }; >>>>> #endif >>>>> >>>>> +static int hdmiphy_probe(struct i2c_client *client, >>>>> + const struct i2c_device_id *id) >>>>> +{ >>>>> + struct device *dev = &client->dev; >>>>> + struct hdmiphy_context *hdata; >>>>> + struct exynos_drm_hdmi_context *drm_hdmi_ctx; >>>>> + >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + drm_hdmi_ctx = devm_kzalloc(dev, sizeof(*drm_hdmi_ctx), >>>>> + GFP_KERNEL); >>>>> + if (!drm_hdmi_ctx) { >>>>> + DRM_ERROR("failed to allocate common hdmi context.\n"); >>>>> + return -ENOMEM; >>>>> + } >>>>> + >>>>> + hdata = devm_kzalloc(dev, sizeof(*hdata), GFP_KERNEL); >>>>> + if (!hdata) { >>>>> + DRM_ERROR("failed to allocate hdmiphy context.\n"); >>>>> + return -ENOMEM; >>>>> + } >>>>> + >>>>> + if (dev->of_node) { >>>>> + const struct of_device_id *match; >>>>> + match = of_match_node(of_match_ptr(hdmiphy_match_types), >>>>> + dev->of_node); >>>>> + if (match == NULL) >>>>> + return -ENODEV; >>>>> + hdata->type = (enum hdmiphy_type)match->data; >>>>> + } else { >>>>> + hdata->type = (enum hdmiphy_type)id->driver_data; >>>>> + } >>>>> + >>>>> + drm_hdmi_ctx->ctx = (void *)hdata; >>>>> + hdata->parent_ctx = (void *)drm_hdmi_ctx; >>>>> + hdata->dev = dev; >>>>> + >>>>> + hdata->hdmiphy = devm_clk_get(dev, "hdmiphy"); >>>> >>>> I don't see this call removed in the hdmi driver, am I missing something? >>>> >>> >>> I took it wrong. I thought hdmi IP is clocked by hdmiphy clock. Actually it is >>> sclk_hdmiphy clock. I will remove this from hdmi driver. >>> >>>>> + if (IS_ERR_OR_NULL(hdata->hdmiphy)) { >>>>> + DRM_ERROR("failed to get clock 'hdmiphy'\n"); >>>>> + return PTR_ERR(hdata->hdmiphy); >>>>> + } >>>>> + >>>>> + i2c_set_clientdata(client, hdata); >>>>> + >>>>> + /* Attach HDMI-PHY Driver to common hdmi. */ >>>>> + exynos_hdmiphy_drv_attach(drm_hdmi_ctx); >>>>> + >>>>> + /* register specific callbacks to common hdmi. */ >>>>> + exynos_hdmiphy_ops_register(&hdmiphy_ops); >>>>> + >>>>> + pm_runtime_enable(dev); >>>>> + >>>>> + dev_info(&client->adapter->dev, >>>>> + "attached s5p_hdmiphy into i2c adapter successfully\n"); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int hdmiphy_remove(struct i2c_client *client) >>>>> +{ >>>>> + dev_info(&client->adapter->dev, >>>>> + "detached s5p_hdmiphy from i2c adapter successfully\n"); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static void hdmiphy_poweroff(struct device *dev) >>>>> +{ >>>>> + struct i2c_client *client = to_i2c_client(dev); >>>>> + struct hdmiphy_context *hdata = i2c_get_clientdata(client); >>>>> + >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + if (hdata->type == HDMIPHY_EXYNOS4212) >>>>> + hdmiphy_4412_turn_on(client, 0); >>>>> + >>>>> + clk_disable(hdata->hdmiphy); >>>>> +} >>>>> + >>>>> +static void hdmiphy_poweron(struct device *dev) >>>>> +{ >>>>> + struct i2c_client *client = to_i2c_client(dev); >>>>> + struct hdmiphy_context *hdata = i2c_get_clientdata(client); >>>>> + >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + clk_enable(hdata->hdmiphy); >>>>> + >>>>> + if (hdata->type == HDMIPHY_EXYNOS4212) >>>>> + hdmiphy_4412_turn_on(client, 1); >>>>> +} >>>>> + >>>>> +#ifdef CONFIG_PM_SLEEP >>>>> +static int hdmiphy_suspend(struct device *dev) >>>>> +{ >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + if (pm_runtime_suspended(dev)) { >>>>> + DRM_DEBUG_KMS("%s: already runtime-suspended.\n", >>>>> + __func__); >>>>> + return 0; >>>>> + } >>>>> + >>>>> + hdmiphy_poweroff(dev); >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int hdmiphy_resume(struct device *dev) >>>>> +{ >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + if (pm_runtime_suspended(dev)) { >>>>> + /* dpms callback should resume the mixer. */ >>>>> + DRM_DEBUG_KMS("%s: already runtime-suspended.\n", >>>>> + __func__); >>>>> + return 0; >>>>> + } >>>>> + >>>>> + hdmiphy_poweron(dev); >>>>> + return 0; >>>>> +} >>>>> +#endif >>>>> + >>>>> + >>>>> +#ifdef CONFIG_PM_RUNTIME >>>>> +static int hdmiphy_runtime_suspend(struct device *dev) >>>>> +{ >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + hdmiphy_poweroff(dev); >>>>> + return 0; >>>>> +} >>>>> + >>>>> +static int hdmiphy_runtime_resume(struct device *dev) >>>>> +{ >>>>> + DRM_DEBUG_KMS("[%d] %s\n", __LINE__, __func__); >>>>> + >>>>> + hdmiphy_poweron(dev); >>>>> + return 0; >>>>> +} >>>>> +#endif >>>>> + >>>>> +static const struct dev_pm_ops hdmiphy_pm_ops = { >>>>> + SET_SYSTEM_SLEEP_PM_OPS(hdmiphy_suspend, hdmiphy_resume) >>>>> + SET_RUNTIME_PM_OPS(hdmiphy_runtime_suspend, >>>>> + hdmiphy_runtime_resume, NULL) >>>>> +}; >>>>> + >>>>> struct i2c_driver hdmiphy_driver = { >>>>> .driver = { >>>>> .name = "exynos-hdmiphy", >>>>> .owner = THIS_MODULE, >>>>> .of_match_table = of_match_ptr(hdmiphy_match_types), >>>>> + .pm = &hdmiphy_pm_ops, >>>>> }, >>>>> .id_table = hdmiphy_id, >>>>> .probe = hdmiphy_probe, >>>>> .remove = hdmiphy_remove, >>>>> .command = NULL, >>>>> }; >>>>> + >>>>> +extern int exynos_hdmiphy_driver_register(void) >>>>> +{ >>>>> + return i2c_add_driver(&hdmiphy_driver); >>>>> +} >>>>> + >>>>> +extern void exynos_hdmiphy_driver_unregister(void) >>>>> +{ >>>>> + i2c_del_driver(&hdmiphy_driver); >>>>> +} >>>>> + >>>>> EXPORT_SYMBOL(hdmiphy_driver); >>>>> diff --git a/drivers/gpu/drm/exynos/regs-hdmiphy.h b/drivers/gpu/drm/exynos/regs-hdmiphy.h >>>>> new file mode 100644 >>>>> index 0000000..8f906c6 >>>>> --- /dev/null >>>>> +++ b/drivers/gpu/drm/exynos/regs-hdmiphy.h >>>>> @@ -0,0 +1,61 @@ >>>>> +/* >>>>> + * >>>>> + * regs-hdmiphy.h >>>>> + * >>>>> + * Copyright (c) 2013 Samsung Electronics Co., Ltd. >>>>> + * http://www.samsung.com/ >>>>> + * >>>>> + * HDMI-PHY register header file for Samsung TVOUT driver >>>>> + * >>>>> + * This program is free software; you can redistribute it and/or modify >>>>> + * it under the terms of the GNU General Public License version 2 as >>>>> + * published by the Free Software Foundation. >>>>> +*/ >>>>> + >>>>> +#ifndef SAMSUNG_REGS_HDMIPHY_H >>>>> +#define SAMSUNG_REGS_HDMIPHY_H >>>>> + >>>>> +/* >>>>> + * Register part >>>>> +*/ >>>>> + >>>>> +/* HDMI PHY Version Common */ >>>>> +#define HDMIPHY_MODE_SET_DONE (0x1f) >>>>> + >>>>> +/* HDMI PHY Version 4212 */ >>>>> +#define HDMIPHY_4212_PCG_CON (0x04) >>>>> +#define HDMIPHY_4212_OSC_PAD_CON (0x0b) >>>>> +#define HDMIPHY_4212_PD_CON (0x1d) >>>>> + >>>>> +/* >>>>> + * Bit definition part >>>>> + */ >>>>> + >>>>> +/* HDMIPHY_MODE_SET_DONE */ >>>>> +#define HDMIPHY_MODE_EN (1 << 7) >>>>> + >>>>> +/* HDMIPHY_4212_PCG_CON */ >>>>> +#define HDMIPHY_PCG_RESET_EN (1 << 3) >>>>> + >>>>> +/* HDMIPHY_4212_OSC_PAD_CON */ >>>>> +#define HDMIPHY_OSC_PAD_EN (3 << 6) >>>>> + >>>>> +/* HDMIPHY_4212_PD_CON */ >>>>> +#define HDMIPHY_PDEN (1 << 7) >>>>> + >>>>> +#define HDMIPHY_PLL_PD (1 << 6) >>>>> +#define HDMIPHY_CLKSER_PD (1 << 5) >>>>> +#define HDMIPHY_CLKDRV_PD (1 << 4) >>>>> + >>>>> +#define HDMIPHY_DRV_PD (1 << 2) >>>>> +#define HDMIPHY_SER_PD (1 << 1) >>>>> +#define HDMIPHY_ICLK_PD (1 << 0) >>>>> + >>>>> +#define HDMIPHY_PD_ALL (HDMIPHY_PLL_PD |\ >>>>> + HDMIPHY_CLKSER_PD |\ >>>>> + HDMIPHY_CLKDRV_PD|\ >>>>> + HDMIPHY_DRV_PD|\ >>>>> + HDMIPHY_SER_PD|\ >>>>> + HDMIPHY_ICLK_PD) >>>>> + >>>>> +#endif /* SAMSUNG_REGS_HDMIPHY_H */ >>>>> -- >>>>> 1.8.0 >>>>> >>> >>> -- >>> Regards, >>> Rahul Sharma, >>> email to: rahul.sharma@xxxxxxxxxxx >>> Samsung India. >> >> >> >> -- >> Regards, >> Rahul Sharma, >> email to: rahul.sharma@xxxxxxxxxxx >> Samsung India. >> _______________________________________________ >> dri-devel mailing list >> dri-devel@xxxxxxxxxxxxxxxxxxxxx >> http://lists.freedesktop.org/mailman/listinfo/dri-devel -- Regards, Rahul Sharma, email to: rahul.sharma@xxxxxxxxxxx Samsung India. _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
